Merge pull request #335 from presidentbeef/use_exceptions_instead_of_abort

Use exceptions instead of abort in brakeman.rb

CHANGES

+ * Use exceptions instead of abort in brakeman lib
+
 # 1.9.5
 
  * Add check for unsafe symbol creation

bin/brakeman

@@ -56,22 +56,23 @@ if options[:quiet].nil?
   options[:quiet] = :command_line
 end
 
-if options[:previous_results_json]
-  vulns = Brakeman.compare options.merge(:quiet => options[:quiet])
-  puts MultiJson.dump(vulns, :pretty => true)
+begin
+  if options[:previous_results_json]
+    vulns = Brakeman.compare options.merge(:quiet => options[:quiet])
+    puts MultiJson.dump(vulns, :pretty => true)
 
-  if options[:exit_on_warn] and (vulns[:new].count + vulns[:fixed].count > 0)
-    exit Brakeman::Warnings_Found_Exit_Code
-  end
-else
-  #Run scan and output a report
-  tracker = Brakeman.run options.merge(:print_report => true, :quiet => options[:quiet])
+    if options[:exit_on_warn] and (vulns[:new].count + vulns[:fixed].count > 0)
+      exit Brakeman::Warnings_Found_Exit_Code
+    end
+  else
+    #Run scan and output a report
+    tracker = Brakeman.run options.merge(:print_report => true, :quiet => options[:quiet])
 
-  #Return error code if --exit-on-warn is used and warnings were found
-  if options[:exit_on_warn] and not tracker.checks.all_warnings.empty?
-    exit Brakeman::Warnings_Found_Exit_Code
+    #Return error code if --exit-on-warn is used and warnings were found
+    if options[:exit_on_warn] and not tracker.checks.all_warnings.empty?
+      exit Brakeman::Warnings_Found_Exit_Code
+    end
   end
+rescue Brakeman::Scanner::NoApplication => e
+  $stderr.puts e.message
 end
-
-
-

lib/brakeman.rb

@@ -195,11 +195,19 @@ module Brakeman
   #Installs Rake task for running Brakeman,
   #which basically means copying `lib/brakeman/brakeman.rake` to
   #`lib/tasks/brakeman.rake` in the current Rails application.
-  def self.install_rake_task
-    if not File.exists? "Rakefile"
-      abort "No Rakefile detected"
-    elsif File.exists? "lib/tasks/brakeman.rake"
-      abort "Task already exists"
+  def self.install_rake_task install_path = nil
+    if install_path
+      rake_path = File.join(install_path, "Rakefile")
+      task_path = File.join(install_path, "lib", "tasks", "brakeman.rake")
+    else
+      rake_path = "Rakefile"
+      task_path = File.join("lib", "tasks", "brakeman.rake")
+    end
+
+    if not File.exists? rake_path
+      raise RakeInstallError, "No Rakefile detected"
+    elsif File.exists? task_path
+      raise RakeInstallError, "Task already exists"
     end
 
     require 'fileutils'
@@ -211,13 +219,13 @@ module Brakeman
 
     path = File.expand_path(File.dirname(__FILE__))
 
-    FileUtils.cp "#{path}/brakeman/brakeman.rake", "lib/tasks/brakeman.rake"
+    FileUtils.cp "#{path}/brakeman/brakeman.rake", task_path
 
-    if File.exists? "lib/tasks/brakeman.rake"
-      notify "Task created in lib/tasks/brakeman.rake"
+    if File.exists? task_path
+      notify "Task created in #{task_path}"
       notify "Usage: rake brakeman:run[output_file]"
     else
-      notify "Could not create task"
+      raise RakeInstallError, "Could not create task"
     end
   end
 
@@ -256,7 +264,7 @@ module Brakeman
     begin
       require 'brakeman/scanner'
     rescue LoadError
-      abort "Cannot find lib/ directory."
+      raise NoBrakemanError, "Cannot find lib/ directory."
     end
 
     #Start scanning
@@ -351,4 +359,7 @@ module Brakeman
 
     Brakeman::Differ.new(new_results, previous_results).diff
   end
+
+  class RakeInstallError < RuntimeError; end
+  class NoBrakemanError < RuntimeError; end
 end

lib/brakeman/scanner.rb

@@ -33,7 +33,7 @@ class Brakeman::Scanner
     @app_tree = Brakeman::AppTree.from_options(options)
 
     if !@app_tree.root || !@app_tree.exists?("app")
-      abort("Please supply the path to a Rails application.")
+      raise NoApplication, "Please supply the path to a Rails application."
     end
 
     if @app_tree.exists?("script/rails")
@@ -355,4 +355,6 @@ class Brakeman::Scanner
   def parse_ruby input
     @ruby_parser.new.parse input
   end
+
+  class NoApplication < RuntimeError; end
 end

test/tests/test_brakeman.rb

 require 'tempfile'
 
+class BrakemanTests < Test::Unit::TestCase
+  def test_exception_on_no_application
+    assert_raise Brakeman::Scanner::NoApplication do
+      Brakeman.run "/tmp#{rand}" #better not exist
+    end
+  end
+end
+
 class UtilTests < Test::Unit::TestCase
   def setup
     @ruby_parser = RubyParser

test/tests/test_rake_task.rb

@@ -14,17 +14,20 @@ class RakeTaskTests < Test::Unit::TestCase
 
   def in_temp_app
     Dir.mktmpdir do |dir|
-      FileUtils.cp_r "#{TEST_PATH}/apps/rails3.2/.", dir
+      begin
+        FileUtils.cp_r "#{TEST_PATH}/apps/rails3.2/.", dir
 
-      @rake_task = "#{dir}/lib/tasks/brakeman.rake"
-      @rakefile = "#{dir}/Rakefile"
+        @rake_task = "#{dir}/lib/tasks/brakeman.rake"
+        @rakefile = "#{dir}/Rakefile"
 
-      current_dir = FileUtils.pwd
-      FileUtils.cd dir
+        current_dir = FileUtils.pwd
+        FileUtils.cd dir
 
-      yield dir
+        yield dir
 
-      FileUtils.cd current_dir
+      ensure
+        FileUtils.cd current_dir
+      end
     end
   end
 
@@ -40,11 +43,11 @@ class RakeTaskTests < Test::Unit::TestCase
 
   def test_rake_task_exists
     in_temp_app do
-      assert_nothing_raised SystemExit do
+      assert_nothing_raised Brakeman::RakeInstallError do
         Brakeman.install_rake_task
       end
 
-      assert_raise SystemExit do
+      assert_raise Brakeman::RakeInstallError do
         Brakeman.install_rake_task
       end
     end
@@ -54,7 +57,7 @@ class RakeTaskTests < Test::Unit::TestCase
     in_temp_app do
       File.delete @rakefile
 
-      assert_raise SystemExit do
+      assert_raise Brakeman::RakeInstallError do
         Brakeman.install_rake_task
       end
     end