Replace all `which command` calls

The trouble with `which command` is twofold:
 - It spawns a whole shell just to find out the path to a binary, every time;
 - The results are environment-dependent, since they depend on $PATH. It would
   be better to specify the search path in the configuration file where everything
   else is specified rather than in the environment.

This commit replaces it with the new mechanism from AlaveteliExternalCommand.

2 files changed, 10 insertions, 14 deletions

diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb
index 46993c1a2..0b4ce7b27 100644
--- a/app/models/incoming_message.rb
+++ b/app/models/incoming_message.rb
@@ -266,11 +266,7 @@ class IncomingMessage < ActiveRecord::Base
         # Special cases for some content types
         if content_type == 'application/pdf'
             uncompressed_text = nil
-            IO.popen("#{`which pdftk`.chomp} - output - uncompress", "r+") do |child|
-                child.write(text)
-                child.close_write()
-                uncompressed_text = child.read()
-            end
+            uncompressed_text = AlaveteliExternalCommand.run("pdftk", "-", "output", "-", "uncompress", :stdin_string => text)
             # if we managed to uncompress the PDF...
             if !uncompressed_text.nil? && !uncompressed_text.empty?
                 # then censor stuff (making a copy so can compare again in a bit)
@@ -283,7 +279,7 @@ class IncomingMessage < ActiveRecord::Base
                     if MySociety::Config.get('USE_GHOSTSCRIPT_COMPRESSION') == true
                         command = "gs -sDEVICE=pdfwrite -dCompatibilityLevel=1.4 -dPDFSETTINGS=/screen -dNOPAUSE -dQUIET -dBATCH -sOutputFile=- -"
                     else
-                        command = "#{`which pdftk`.chomp} - output - compress"
+                        command = "#{"pdftk"} - output - compress"
                     end
                     IO.popen(command, "r+") do |child|
                         child.write(censored_uncompressed_text)
@@ -936,22 +932,22 @@ class IncomingMessage < ActiveRecord::Base
             tempfile.print body
             tempfile.flush
             if content_type == 'application/vnd.ms-word'
-                AlaveteliExternalCommand.run(`which wvText`.chomp, tempfile.path, tempfile.path + ".txt")
+                AlaveteliExternalCommand.run("wvText", tempfile.path, tempfile.path + ".txt")
                 # Try catdoc if we get into trouble (e.g. for InfoRequestEvent 2701)
                 if not File.exists?(tempfile.path + ".txt")
-                    AlaveteliExternalCommand.run(`which catdoc`.chomp, tempfile.path, :append_to => text)
+                    AlaveteliExternalCommand.run("catdoc", tempfile.path, :append_to => text)
                 else
                     text += File.read(tempfile.path + ".txt") + "\n\n"
                     File.unlink(tempfile.path + ".txt")
                 end
             elsif content_type == 'application/rtf'
                 # catdoc on RTF prodcues less comments and extra bumf than --text option to unrtf
-                AlaveteliExternalCommand.run(`which catdoc`.chomp, tempfile.path, :append_to => text)
+                AlaveteliExternalCommand.run("catdoc", tempfile.path, :append_to => text)
             elsif content_type == 'text/html'
                 # lynx wordwraps links in its output, which then don't
                 # get formatted properly by Alaveteli. We use elinks
                 # instead, which doesn't do that.
-                AlaveteliExternalCommand.run(`which elinks`.chomp, "-eval", "'set document.codepage.assume = \"#{charset}\"'", "-eval", "'set document.codepage.force_assumed = 1'", "-dump-charset", "utf-8", "-force-html", "-dump",
+                AlaveteliExternalCommand.run("elinks", "-eval", "'set document.codepage.assume = \"#{charset}\"'", "-eval", "'set document.codepage.force_assumed = 1'", "-dump-charset", "utf-8", "-force-html", "-dump",
                     tempfile.path, :append_to => text)
             elsif content_type == 'application/vnd.ms-excel'
                 # Bit crazy using /usr/bin/strings - but xls2csv, xlhtml and
@@ -962,9 +958,9 @@ class IncomingMessage < ActiveRecord::Base
             elsif content_type == 'application/vnd.ms-powerpoint'
                 # ppthtml seems to catch more text, but only outputs HTML when
                 # we want text, so just use catppt for now
-                AlaveteliExternalCommand.run(`which catppt`.chomp, tempfile.path, :append_to => text)
+                AlaveteliExternalCommand.run("catppt", tempfile.path, :append_to => text)
             elsif content_type == 'application/pdf'
-                AlaveteliExternalCommand.run(`which pdftotext`.chomp, tempfile.path, "-", :append_to => text)
+                AlaveteliExternalCommand.run("pdftotext", tempfile.path, "-", :append_to => text)
             elsif content_type == 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'
                 # This is Microsoft's XML office document format.
                 # Just pull out the main XML file, and strip it of text.
diff --git a/lib/alaveteli_external_command.rb b/lib/alaveteli_external_command.rb
index e83d8d49a..737b48fa9 100644
--- a/lib/alaveteli_external_command.rb
+++ b/lib/alaveteli_external_command.rb
@@ -23,14 +23,14 @@ module AlaveteliExternalCommand
                         break
                     end
                 end
-                raise "Could not find #{program_name} in any of #{utility_search_path.join(', ')}" if !found
+                 raise "Could not find #{program_name} in any of #{utility_search_path.join(', ')}" if !found
             end
             
             xc = ExternalCommand.new(program_path, *args)
             if opts.has_key? :append_to
                 xc.out = opts[:append_to]
             end
-            xc.run()
+            xc.run(opts[:stdin_string])
             if xc.status != 0
                 # Error
                 $stderr.puts("Error from #{program_name} #{args.join(' ')}:")