Merge pull request #501 from presidentbeef/parse_files_upfront_instead_of_on_demand

Read/parse files up front

lib/brakeman/file_parser.rb

+module Brakeman
+  ASTFile = Struct.new(:path, :ast)
+
+  # This class handles reading and parsing files.
+  class FileParser
+    attr_reader :file_list
+
+    def initialize tracker, app_tree
+      @tracker = tracker
+      @app_tree = app_tree
+      @file_list = {}
+    end
+
+    def parse_files list, type
+      read_files list, type do |path, contents|
+        if ast = parse_ruby(contents, path)
+          ASTFile.new(path, ast)
+        end
+      end
+    end
+
+    def read_files list, type
+      @file_list[type] ||= []
+
+      list.each do |path|
+        result = yield path, read_path(path)
+        if result
+          @file_list[type] << result
+        end
+      end
+    end
+
+    def parse_ruby input, path
+      begin
+        RubyParser.new.parse input, path
+      rescue Racc::ParseError => e
+        @tracker.error e, "Could not parse #{path}"
+        nil
+      rescue => e
+        @tracker.error e.exception(e.message + "\nWhile processing #{path}"), e.backtrace
+        nil
+      end
+    end
+
+    def read_path path
+      @app_tree.read_path path
+    end
+  end
+end

lib/brakeman/parsers/template_parser.rb

+module Brakeman
+  class TemplateParser
+    include Brakeman::Util
+    attr_reader :tracker
+    KNOWN_TEMPLATE_EXTENSIONS = /.*\.(erb|haml|rhtml|slim)$/
+
+    TemplateFile = Struct.new(:path, :ast, :name, :type)
+
+    def initialize tracker, file_parser
+      @tracker = tracker
+      @file_parser = file_parser
+      @file_parser.file_list[:templates] ||= []
+    end
+
+    def parse_template path, text
+      type = path.match(KNOWN_TEMPLATE_EXTENSIONS)[1].to_sym
+      type = :erb if type == :rhtml
+      name = template_path_to_name path
+
+      begin
+        src = case type
+              when :erb
+                type = :erubis if erubis?
+                parse_erb text
+              when :haml
+                parse_haml text
+              when :slim
+                parse_slim text
+              else
+                tracker.error "Unkown template type in #{path}"
+                nil
+              end
+
+        if src and ast = @file_parser.parse_ruby(src, path)
+          @file_parser.file_list[:templates] << TemplateFile.new(path, ast, name, type)
+        end
+      rescue Racc::ParseError => e
+        tracker.error e, "could not parse #{path}"
+      rescue Haml::Error => e
+        tracker.error e, ["While compiling HAML in #{path}"] << e.backtrace
+      rescue StandardError, LoadError => e
+        tracker.error e.exception(e.message + "\nWhile processing #{path}"), e.backtrace
+      end
+
+      nil
+    end
+
+    def parse_erb text
+      if tracker.config[:escape_html]
+        if tracker.options[:rails3]
+          require 'brakeman/parsers/rails3_erubis'
+          Brakeman::Rails3Erubis.new(text).src
+        else
+          require 'brakeman/parsers/rails2_xss_plugin_erubis'
+          Brakeman::Rails2XSSPluginErubis.new(text).src
+        end
+      elsif tracker.config[:erubis]
+        require 'brakeman/parsers/rails2_erubis'
+        Brakeman::ScannerErubis.new(text).src
+      else
+        require 'erb'
+        src = ERB.new(text, nil, "-").src
+        src.sub!(/^#.*\n/, '') if Brakeman::Scanner::RUBY_1_9
+        src
+      end
+    end
+
+    def erubis?
+      tracker.config[:escape_html] or
+      tracker.config[:erubis]
+    end
+
+    def parse_haml text
+      Brakeman.load_brakeman_dependency 'haml'
+      Brakeman.load_brakeman_dependency 'sass'
+
+      Haml::Engine.new(text,
+                       :escape_html => !!tracker.config[:escape_html]).precompiled
+    end
+
+    def parse_slim text
+      Brakeman.load_brakeman_dependency 'slim'
+
+      Slim::Template.new(:disable_capture => true,
+                         :generator => Temple::Generators::RailsOutputBuffer) { text }.precompiled_template
+    end
+  end
+end

lib/brakeman/rescanner.rb

@@ -5,7 +5,8 @@ require 'brakeman/differ'
 
 #Class for rescanning changed files after an initial scan
 class Brakeman::Rescanner < Brakeman::Scanner
-
+ include Brakeman::Util
+  KNOWN_TEMPLATE_EXTENSIONS = Brakeman::TemplateParser::KNOWN_TEMPLATE_EXTENSIONS
   SCAN_ORDER = [:config, :gemfile, :initializer, :lib, :routes, :template,
     :model, :controller]
 
@@ -83,16 +84,9 @@ class Brakeman::Rescanner < Brakeman::Scanner
     when :config
       process_config
     when :initializer
-      process_initializer path
+      rescan_initializer path
     when :routes
-      # Routes affect which controller methods are treated as actions
-      # which affects which templates are rendered, so routes, controllers,
-      # and templates rendered from controllers must be rescanned
-      tracker.reset_routes
-      tracker.reset_templates :only_rendered => true
-      process_routes
-      process_controllers
-      @reindex << :controllers << :templates
+      rescan_routes
     when :gemfile
       if tracker.config[:gems][:rails_xss] and tracker.config[:escape_html]
         tracker.config[:escape_html] = false
@@ -108,8 +102,10 @@ class Brakeman::Rescanner < Brakeman::Scanner
 
   def rescan_controller path
     controller = tracker.reset_controller path
-    paths =  controller.nil? ? [path] : controller[:files]
-    paths.each { |path| process_controller path if @app_tree.path_exists?(path)  }
+    paths = controller.nil? ? [path] : controller[:files]
+    parse_ruby_files(paths).each do |astfile|
+      process_controller astfile
+    end
 
     #Process data flow and template rendering
     #from the controller
@@ -122,7 +118,7 @@ class Brakeman::Rescanner < Brakeman::Scanner
           end
         end
 
-        controller[:src].values.each do |src|
+        controller[:src].each_value do |src|
           @processor.process_controller_alias controller[:name], src
         end
       end
@@ -137,7 +133,10 @@ class Brakeman::Rescanner < Brakeman::Scanner
     template_name = template_path_to_name(path)
 
     tracker.reset_template template_name
-    process_template path
+    fp = Brakeman::FileParser.new(tracker, @app_tree)
+    template_parser = Brakeman::TemplateParser.new(tracker, fp)
+    template_parser.parse_template path, @app_tree.read_path(path)
+    process_template fp.file_list[:templates].first
 
     @processor.process_template_alias tracker.templates[template_name]
 
@@ -186,12 +185,14 @@ class Brakeman::Rescanner < Brakeman::Scanner
     num_models = tracker.models.length
     model = tracker.reset_model path
     paths = model.nil? ? [path] : model[:files]
-    paths.each { |path| process_model path if @app_tree.path_exists?(path)  }
+    parse_ruby_files(paths).each do |astfile|
+      process_model astfile.path, astfile.ast
+    end
 
     #Only need to rescan other things if a model is added or removed
     if num_models != tracker.models.length
-      process_templates
-      process_controllers
+      process_template_data_flows
+      process_controller_data_flows
       @reindex << :templates << :controllers
     end
 
@@ -201,7 +202,9 @@ class Brakeman::Rescanner < Brakeman::Scanner
   def rescan_lib path
     lib = tracker.reset_lib path
     paths = lib.nil? ? [path] : lib[:files]
-    paths.each { |path| process_lib path if @app_tree.path_exists?(path)  }
+    parse_ruby_files(paths).each do |astfile|
+      process_lib astfile
+    end
 
     lib = nil
 
@@ -215,6 +218,23 @@ class Brakeman::Rescanner < Brakeman::Scanner
     rescan_mixin lib if lib
   end
 
+  def rescan_routes
+    # Routes affect which controller methods are treated as actions
+    # which affects which templates are rendered, so routes, controllers,
+    # and templates rendered from controllers must be rescanned
+    tracker.reset_routes
+    tracker.reset_templates :only_rendered => true
+    process_routes
+    process_controller_data_flows
+    @reindex << :controllers << :templates
+  end
+
+  def rescan_initializer path
+    parse_ruby_files([path]).each do |astfile|
+      process_initializer astfile
+    end
+  end
+
   #Handle rescanning when a file is deleted
   def rescan_deleted_file path, type
     case type
@@ -380,6 +400,13 @@ class Brakeman::Rescanner < Brakeman::Scanner
       rescan_file template[1]
     end
   end
+
+  def parse_ruby_files list
+    paths = list.select { |path| @app_tree.path_exists? path }
+    file_parser = Brakeman::FileParser.new(tracker, @app_tree)
+    file_parser.parse_files paths, :rescan
+    file_parser.file_list[:rescan]
+  end
 end
 
 #Class to make reporting of rescan results simpler to deal with

lib/brakeman/scanner.rb

@@ -6,6 +6,8 @@ begin
   require 'ruby_parser/bm_sexp_processor.rb'
   require 'brakeman/processor'
   require 'brakeman/app_tree'
+  require 'brakeman/file_parser'
+  require 'brakeman/parsers/template_parser'
 rescue LoadError => e
   $stderr.puts e.message
   $stderr.puts "Please install the appropriate dependency."
@@ -15,9 +17,7 @@ end
 #Scans the Rails application.
 class Brakeman::Scanner
   attr_reader :options
-
-  RUBY_1_9 = !!(RUBY_VERSION >= "1.9.0")
-  KNOWN_TEMPLATE_EXTENSIONS = /.*\.(erb|haml|rhtml|slim)$/
+  RUBY_1_9 = RUBY_VERSION >= "1.9.0"
 
   #Pass in path to the root of the Rails application
   def initialize options, processor = nil
@@ -36,7 +36,6 @@ class Brakeman::Scanner
       Brakeman.notify "[Notice] Detected Rails 4 application"
     end
 
-    @ruby_parser = ::RubyParser
     @processor = processor || Brakeman::Processor.new(@app_tree, options)
   end
 
@@ -51,6 +50,8 @@ class Brakeman::Scanner
     process_gems
     Brakeman.notify "Processing configuration..."
     process_config
+    Brakeman.notify "Parsing files..."
+    parse_files
     Brakeman.notify "Processing initializers..."
     process_initializers
     Brakeman.notify "Processing libs..."
@@ -59,15 +60,45 @@ class Brakeman::Scanner
     process_routes
     Brakeman.notify "Processing templates...       "
     process_templates
+    Brakeman.notify "Processing data flow in templates..."
+    process_template_data_flows
     Brakeman.notify "Processing models...          "
     process_models
     Brakeman.notify "Processing controllers...     "
     process_controllers
+    Brakeman.notify "Processing data flow in controllers..."
+    process_controller_data_flows
     Brakeman.notify "Indexing call sites...        "
     index_call_sites
     tracker
   end
 
+  def parse_files
+    fp = Brakeman::FileParser.new tracker, @app_tree
+
+    files = {
+      :initializers => @app_tree.initializer_paths,
+      :controllers => @app_tree.controller_paths,
+      :models => @app_tree.model_paths
+    }
+
+    unless options[:skip_libs]
+      files[:libs] = @app_tree.lib_paths
+    end
+
+    files.each do |name, paths|
+      fp.parse_files paths, name
+    end
+
+    template_parser = Brakeman::TemplateParser.new(tracker, fp)
+
+    fp.read_files(@app_tree.template_paths, :templates) do |path, contents|
+      template_parser.parse_template path, contents
+    end
+
+    @file_list = fp.file_list
+  end
+
   #Process config/environment.rb and config/gems.rb
   #
   #Stores parsed information in tracker.config
@@ -120,20 +151,15 @@ class Brakeman::Scanner
   #
   #Adds parsed information to tracker.initializers
   def process_initializers
-    @app_tree.initializer_paths.each do |f|
-      process_initializer f
+    track_progress @file_list[:initializers] do |init|
+      Brakeman.debug "Processing #{init[:path]}"
+      process_initializer init
     end
   end
 
   #Process an initializer
-  def process_initializer path
-    begin
-      @processor.process_initializer(path, parse_ruby(@app_tree.read_path(path)))
-    rescue Racc::ParseError => e
-      tracker.error e, "could not parse #{path}. There is probably a typo in the file. Test it with 'ruby_parse #{path}'"
-    rescue => e
-      tracker.error e.exception(e.message + "\nWhile processing #{path}"), e.backtrace
-    end
+  def process_initializer init
+    @processor.process_initializer(init.path, init.ast)
   end
 
   #Process all .rb in lib/
@@ -145,26 +171,15 @@ class Brakeman::Scanner
       return
     end
 
-    total = @app_tree.lib_paths.length
-    current = 0
-
-    @app_tree.lib_paths.each do |f|
-      Brakeman.debug "Processing #{f}"
-      report_progress(current, total)
-      current += 1
-      process_lib f
+    track_progress @file_list[:libs] do |lib|
+      Brakeman.debug "Processing #{lib.path}"
+      process_lib lib
     end
   end
 
   #Process a library
-  def process_lib path
-    begin
-      @processor.process_lib parse_ruby(@app_tree.read_path(path)), path
-    rescue Racc::ParseError => e
-      tracker.error e, "could not parse #{path}. There is probably a typo in the file. Test it with 'ruby_parse #{path}'"
-    rescue => e
-      tracker.error e.exception(e.message + "\nWhile processing #{path}"), e.backtrace
-    end
+  def process_lib lib
+    @processor.process_lib lib.ast, lib.path
   end
 
   #Process config/routes.rb
@@ -188,26 +203,18 @@ class Brakeman::Scanner
   #
   #Adds processed controllers to tracker.controllers
   def process_controllers
-    total = @app_tree.controller_paths.length
-    current = 0
-
-    @app_tree.controller_paths.each do |f|
-      Brakeman.debug "Processing #{f}"
-      report_progress(current, total)
-      current += 1
-      process_controller f
+    track_progress @file_list[:controllers] do |controller|
+      Brakeman.debug "Processing #{controller.path}"
+      process_controller controller
     end
+  end
 
-    current = 0
-    total = tracker.controllers.length
-
-    Brakeman.notify "Processing data flow in controllers..."
+  def process_controller_data_flows
+    controllers = tracker.controllers.sort_by { |name, _| name.to_s }
 
-    tracker.controllers.sort_by{|name| name.to_s}.each do |name, controller|
+    track_progress controllers, "controllers" do |name, controller|
       Brakeman.debug "Processing #{name}"
-      report_progress(current, total, "controllers")
-      current += 1
-      controller[:src].values.each do |src|
+      controller[:src].each_value do |src|
         @processor.process_controller_alias name, src
       end
     end
@@ -216,11 +223,9 @@ class Brakeman::Scanner
     tracker.filter_cache.clear
   end
 
-  def process_controller path
+  def process_controller astfile
     begin
-      @processor.process_controller(parse_ruby(@app_tree.read_path(path)), path)
-    rescue Racc::ParseError => e
-      tracker.error e, "could not parse #{path}. There is probably a typo in the file. Test it with 'ruby_parse #{path}'"
+      @processor.process_controller(astfile.ast, astfile.path)
     rescue => e
       tracker.error e.exception(e.message + "\nWhile processing #{path}"), e.backtrace
     end
@@ -230,119 +235,48 @@ class Brakeman::Scanner
   #
   #Adds processed views to tracker.views
   def process_templates
-    $stdout.sync = true
-
-    count = 0
-    total = @app_tree.template_paths.length
+    templates = @file_list[:templates].sort_by { |t| t[:path] }
 
-    @app_tree.template_paths.each do |path|
-      Brakeman.debug "Processing #{path}"
-      report_progress(count, total)
-      count += 1
-      process_template path
-    end
-
-    total = tracker.templates.length
-    count = 0
-
-    Brakeman.notify "Processing data flow in templates..."
-
-    tracker.templates.keys.dup.sort_by{|name| name.to_s}.each do |name|
-      Brakeman.debug "Processing #{name}"
-      report_progress(count, total, "templates")
-      count += 1
-      @processor.process_template_alias tracker.templates[name]
+    track_progress templates, "templates" do |template|
+      Brakeman.debug "Processing #{template[:path]}"
+      process_template template
     end
   end
 
-  def process_template path
-    type = path.match(KNOWN_TEMPLATE_EXTENSIONS)[1].to_sym
-    type = :erb if type == :rhtml
-    name = template_path_to_name path
-    text = @app_tree.read_path path
-
-    begin
-      if type == :erb
-        if tracker.config[:escape_html]
-          type = :erubis
-          if options[:rails3]
-            require 'brakeman/parsers/rails3_erubis'
-            src = Brakeman::Rails3Erubis.new(text).src
-          else
-            require 'brakeman/parsers/rails2_xss_plugin_erubis'
-            src = Brakeman::Rails2XSSPluginErubis.new(text).src
-          end
-        elsif tracker.config[:erubis]
-          require 'brakeman/parsers/rails2_erubis'
-          type = :erubis
-          src = Brakeman::ScannerErubis.new(text).src
-        else
-          require 'erb'
-          src = ERB.new(text, nil, "-").src
-          src.sub!(/^#.*\n/, '') if RUBY_1_9
-        end
-
-        parsed = parse_ruby src
-      elsif type == :haml
-        Brakeman.load_brakeman_dependency 'haml'
-        Brakeman.load_brakeman_dependency 'sass'
-
-        src = Haml::Engine.new(text,
-                               :escape_html => !!tracker.config[:escape_html]).precompiled
-        parsed = parse_ruby src
-      elsif type == :slim
-        Brakeman.load_brakeman_dependency 'slim'
-
-        src = Slim::Template.new(:disable_capture => true,
-                                 :generator => Temple::Generators::RailsOutputBuffer) { text }.precompiled_template
-
-        parsed = parse_ruby src
-      else
-        tracker.error "Unkown template type in #{path}"
-      end
+  def process_template template
+    @processor.process_template(template.name, template.ast, template.type, nil, template.path)
+  end
 
-      @processor.process_template(name, parsed, type, nil, path)
+  def process_template_data_flows
+    templates = tracker.templates.sort_by { |name, _| name.to_s }
 
-    rescue Racc::ParseError => e
-      tracker.error e, "could not parse #{path}"
-    rescue Haml::Error => e
-      tracker.error e, ["While compiling HAML in #{path}"] << e.backtrace
-    rescue StandardError, LoadError => e
-      tracker.error e.exception(e.message + "\nWhile processing #{path}"), e.backtrace
+    track_progress templates, "templates" do |name, template|
+      Brakeman.debug "Processing #{name}"
+      @processor.process_template_alias template
     end
   end
 
-  #Convert path/filename to view name
-  #
-  # views/test/something.html.erb -> test/something
-  def template_path_to_name path
-    names = path.split("/")
-    names.last.gsub!(/(\.(html|js)\..*|\.rhtml)$/, '')
-    names[(names.index("views") + 1)..-1].join("/").to_sym
-  end
-
   #Process all the .rb files in models/
   #
   #Adds the processed models to tracker.models
   def process_models
-    total = @app_tree.model_paths.length
-    current = 0
-
-    @app_tree.model_paths.each do |f|
-      Brakeman.debug "Processing #{f}"
-      report_progress(current, total)
-      current += 1
-      process_model f
+    track_progress @file_list[:models] do |model|
+      Brakeman.debug "Processing #{model[:path]}"
+      process_model model[:path], model[:ast]
     end
   end
 
-  def process_model path
-    begin
-      @processor.process_model(parse_ruby(@app_tree.read_path(path)), path)
-    rescue Racc::ParseError => e
-      tracker.error e, "could not parse #{path}"
-    rescue => e
-      tracker.error e.exception(e.message + "\nWhile processing #{path}"), e.backtrace
+  def process_model path, ast
+    @processor.process_model(ast, path)
+  end
+
+  def track_progress list, type = "files"
+    total = list.length
+    current = 0
+    list.each do |item|
+      report_progress current, total, type
+      current += 1
+      yield item
     end
   end
 
@@ -356,7 +290,7 @@ class Brakeman::Scanner
   end
 
   def parse_ruby input
-    @ruby_parser.new.parse input
+    RubyParser.new.parse input
   end
 end
 

lib/brakeman/util.rb

@@ -383,6 +383,15 @@ module Brakeman::Util
     end
   end
 
+  #Convert path/filename to view name
+  #
+  # views/test/something.html.erb -> test/something
+  def template_path_to_name path
+    names = path.split("/")
+    names.last.gsub!(/(\.(html|js)\..*|\.rhtml)$/, '')
+    names[(names.index("views") + 1)..-1].join("/").to_sym
+  end
+
   def github_url file, line=nil
     if repo_url = @tracker.options[:github_url] and file and not file.empty? and file.start_with? '/'
       url = "#{repo_url}/#{relative_path(file)}"