Merge remote-tracking branch 'zlx/zlx/refactor_brakeman'

.gitignore

 Gemfile.lock
 .rbx
 **/.rbx
+coverage/
+test/coverage/

lib/brakeman.rb

@@ -62,16 +62,10 @@ module Brakeman
     if options.is_a? String
       options = { :app_path => options }
     end
+    
+    options = default_options.merge(load_options(options[:config_file], options[:quiet])).merge(options)
 
     options[:app_path] = File.expand_path(options[:app_path])
-
-    file_options = load_options(options[:config_file])
-
-    options = file_options.merge options
-
-    options[:quiet] = true if options[:quiet].nil? && file_options[:quiet]
-
-    options = get_defaults.merge! options
     options[:output_formats] = get_output_formats options
 
     options
@@ -84,12 +78,14 @@ module Brakeman
   ]
 
   #Load options from YAML file
-  def self.load_options custom_location
+  def self.load_options custom_location, quiet
     #Load configuration file
     if config = config_file(custom_location)
       options = YAML.load_file config
       options.each { |k, v| options[k] = Set.new v if v.is_a? Array }
-      notify "[Notice] Using configuration in #{config}" unless options[:quiet]
+      
+      # notify if options[:quiet] and quiet is nil||false
+      notify "[Notice] Using configuration in #{config}" unless (options[:quiet] || quiet)
       options
     else
       {}
@@ -102,7 +98,7 @@ module Brakeman
   end
 
   #Default set of options
-  def self.get_defaults
+  def self.default_options
     { :assume_all_routes => true,
       :skip_checks => Set.new,
       :check_arguments => true,
@@ -130,51 +126,62 @@ module Brakeman
       raise ArgumentError, "Cannot specify output format if multiple output files specified"
     end
     if options[:output_format]
-      [
-        case options[:output_format]
-        when :html, :to_html
-          :to_html
-        when :csv, :to_csv
-          :to_csv
-        when :pdf, :to_pdf
-          :to_pdf
-        when :tabs, :to_tabs
-          :to_tabs
-        when :json, :to_json
-          :to_json
-        else
-          :to_s
-        end
-      ]
+      get_formats_from_output_format options[:output_format]
+    elsif options[:output_files]
+      get_formats_from_output_files options[:output_files]
     else
-      return [:to_s] unless options[:output_files]
-      options[:output_files].map do |output_file|
-        case output_file
-        when /\.html$/i
-          :to_html
-        when /\.csv$/i
-          :to_csv
-        when /\.pdf$/i
-          :to_pdf
-        when /\.tabs$/i
-          :to_tabs
-        when /\.json$/i
-          :to_json
-        else
-          :to_s
-        end
+      return [:to_s]
+    end
+  end
+  
+  def self.get_formats_from_output_format output_format
+    case output_format
+    when :html, :to_html
+      [:to_html]
+    when :csv, :to_csv
+      [:to_csv]
+    when :pdf, :to_pdf
+      [:to_pdf]
+    when :tabs, :to_tabs
+      [:to_tabs]
+    when :json, :to_json
+      [:to_json]
+    else
+      [:to_s]
+    end
+  end
+  private_class_method :get_formats_from_output_format
+  
+  def self.get_formats_from_output_files output_files
+    output_files.map do |output_file|
+      case output_file
+      when /\.html$/i
+        :to_html
+      when /\.csv$/i
+        :to_csv
+      when /\.pdf$/i
+        :to_pdf
+      when /\.tabs$/i
+        :to_tabs
+      when /\.json$/i
+        :to_json
+      else
+        :to_s
       end
     end
   end
+  private_class_method :get_formats_from_output_files
 
   #Output list of checks (for `-k` option)
   def self.list_checks
     require 'brakeman/scanner'
+    format_length = 30
+    
     $stderr.puts "Available Checks:"
-    $stderr.puts "-" * 30
-    $stderr.puts Checks.checks.map { |c|
-      c.to_s.match(/^Brakeman::(.*)$/)[1].ljust(27) << c.description
-    }.sort.join "\n"
+    $stderr.puts "-" * format_length
+    Checks.checks.each do |check|
+      $stderr.printf("%-#{format_length}s%s\n", check.name, check.description)
+    end
   end
 
   #Installs Rake task for running Brakeman,
@@ -260,22 +267,32 @@ module Brakeman
     if options[:output_files]
       notify "Generating report..."
 
-      options[:output_files].each_with_index do |output_file, idx|
-        File.open output_file, "w" do |f|
-          f.write tracker.report.send(options[:output_formats][idx])
-        end
-        notify "Report saved in '#{output_file}'"
-      end
+      write_report_to_files tracker, options[:output_files]
     elsif options[:print_report]
       notify "Generating report..."
 
-      options[:output_formats].each do |output_format|
-        puts tracker.report.send(output_format)
-      end
+      write_report_to_formats tracker, options[:output_formats]
     end
 
     tracker
   end
+  
+  def self.write_report_to_files tracker, output_files
+    output_files.each_with_index do |output_file, idx|
+      File.open output_file, "w" do |f|
+        f.write tracker.report.format(output_file)
+      end
+      notify "Report saved in '#{output_file}'"
+    end
+  end
+  private_class_method :write_report_to_files
+  
+  def self.write_report_to_formats tracker, output_formats
+    output_formats.each do |output_format|
+      puts tracker.report.format(output_format)
+    end
+  end
+  private_class_method :write_report_to_formats
 
   #Rescan a subset of files in a Rails application.
   #

lib/brakeman/checks/base_check.rb

@@ -12,6 +12,14 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
   CONFIDENCE = { :high => 0, :med => 1, :low => 2 }
 
   Match = Struct.new(:type, :match)
+  
+  class << self
+    attr_accessor :name
+  
+    def inherited(subclass)
+      subclass.name = subclass.to_s.match(/^Brakeman::(.*)$/)[1]
+    end
+  end
 
   #Initialize Check with Checks.
   def initialize(app_tree, tracker)

lib/brakeman/report.rb

@@ -283,6 +283,26 @@ class Brakeman::Report
     end
   end
 
+  # format output from filename or format
+  def format(filename_or_format)
+    case filename_or_format
+    when /\.html/, :to_html
+      to_html
+    when /\.pdf/, :to_pdf
+      to_pdf
+    when /\.csv/, :to_csv
+      to_csv
+    when /\.json/, :to_json
+      to_json
+    when /\.tabs/, :to_tabs
+      to_tabs
+    when :to_test
+      to_test
+    else
+      to_s
+    end
+  end
+  
   #Generate HTML output
   def to_html
     out = html_header <<

test/tests/test_brakeman.rb

@@ -55,6 +55,27 @@ class BaseCheckTests < Test::Unit::TestCase
 end
 
 class ConfigTests < Test::Unit::TestCase
+  
+  def setup
+    Brakeman.instance_variable_set(:@quiet, false)
+  end
+  
+  # method from test-unit: http://test-unit.rubyforge.org/test-unit/en/Test/Unit/Util/Output.html#capture_output-instance_method
+  def capture_output
+    require 'stringio'
+
+    output = StringIO.new
+    error = StringIO.new
+    stdout_save, stderr_save = $stdout, $stderr
+    $stdout, $stderr = output, error
+    begin
+      yield
+      [output.string, error.string]
+    ensure
+      $stdout, $stderr = stdout_save, stderr_save
+    end
+  end
+  
   def test_quiet_option_from_file
     config = Tempfile.new("config")
 
@@ -70,11 +91,34 @@ class ConfigTests < Test::Unit::TestCase
       :app_path => "/tmp" #doesn't need to be real
     }
 
-    final_options = Brakeman.set_options(options)
+    assert_equal "", capture_output {
+      final_options = Brakeman.set_options(options)
 
-    config.unlink
+      config.unlink
 
-    assert final_options[:quiet], "Expected quiet option to be true, but was #{final_options[:quiet]}"
+      assert final_options[:quiet], "Expected quiet option to be true, but was #{final_options[:quiet]}"
+    }[1]
+  end
+  
+  def test_quiet_option_from_commandline
+    config = Tempfile.new("config")
+
+    config.write <<-YAML.strip
+    ---
+    app_path: "/tmp"
+    YAML
+
+    config.close
+
+    options = {
+      :config_file => config.path,
+      :quiet => true,
+      :app_path => "/tmp" #doesn't need to be real
+    }
+    
+    assert_equal "", capture_output {
+      final_options = Brakeman.set_options(options)
+    }[1]
   end
 
   def test_quiet_option_default
@@ -97,4 +141,33 @@ class ConfigTests < Test::Unit::TestCase
 
     assert_nil final_options[:quiet]
   end
+  
+  def output_format_tester options, expected_options
+    output_formats = Brakeman.get_output_formats(options)
+    
+    assert_equal expected_options, output_formats
+  end
+  
+  def test_output_format
+    output_format_tester({}, [:to_s])
+    output_format_tester({:output_format => :html}, [:to_html])
+    output_format_tester({:output_format => :to_html}, [:to_html])
+    output_format_tester({:output_format => :csv}, [:to_csv])
+    output_format_tester({:output_format => :to_csv}, [:to_csv])
+    output_format_tester({:output_format => :pdf}, [:to_pdf])
+    output_format_tester({:output_format => :to_pdf}, [:to_pdf])
+    output_format_tester({:output_format => :json}, [:to_json])
+    output_format_tester({:output_format => :to_json}, [:to_json])
+    output_format_tester({:output_format => :tabs}, [:to_tabs])
+    output_format_tester({:output_format => :to_tabs}, [:to_tabs])
+    output_format_tester({:output_format => :others}, [:to_s])
+    
+    output_format_tester({:output_files => ['xx.html', 'xx.pdf']}, [:to_html, :to_pdf])
+    output_format_tester({:output_files => ['xx.pdf', 'xx.json']}, [:to_pdf, :to_json])
+    output_format_tester({:output_files => ['xx.json', 'xx.tabs']}, [:to_json, :to_tabs])
+    output_format_tester({:output_files => ['xx.tabs', 'xx.csv']}, [:to_tabs, :to_csv])
+    output_format_tester({:output_files => ['xx.csv', 'xx.xxx']}, [:to_csv, :to_s])
+    output_format_tester({:output_files => ['xx.xx', 'xx.xx']}, [:to_s, :to_s])
+    output_format_tester({:output_files => ['xx.html', 'xx.pdf', 'xx.csv', 'xx.tabs', 'xx.json']}, [:to_html, :to_pdf, :to_csv, :to_tabs, :to_json])
+  end
 end