mysociety / alaveteli

Provide a Freedom of Information request system for your jurisdiction
https://alaveteli.org
Other
389 stars 196 forks source link

Bug in AlaveteliExternalCommand? #5163

Closed laurentS closed 5 years ago

laurentS commented 5 years ago

Hello!

Not 100% sure this is a bug or my misconfiguration, but I suspect the former. I have about 4 hours of experience with ruby, so it's possible that I'm mistaken, sorry :)

To reproduce

Send a reply to a FOI request in text/html format, then open the request page. The page fails to load, with the following error:

An Errno::ENOENT occurred in request#show:

  No such file or directory - elinks
  lib/alaveteli_external_command.rb:34:in `run'
...
-------------------------------
Backtrace:
-------------------------------

  lib/alaveteli_external_command.rb:34:in `run'
  lib/mail_handler/mail_handler.rb:102:in `get_attachment_text_one_file'
  app/models/incoming_message.rb:486:in `_convert_part_body_to_text'
  app/models/incoming_message.rb:471:in `get_main_body_text_internal'
  app/models/incoming_message.rb:431:in `_cache_main_body_text'
  app/models/incoming_message.rb:463:in `get_main_body_text_unfolded'
  app/models/incoming_message.rb:627:in `get_body_for_html_display'
  app/views/request/_incoming_correspondence.html.erb:23:in `_app_views_request__incoming_correspondence_html_erb___2788323004718852048_70180693217360'
  app/views/request/_correspondence.html.erb:4:in `_app_views_request__correspondence_html_erb__1825183916225692993_70180677010820'
  app/views/request/show.html.erb:70:in `block in _app_views_request_show_html_erb___1752266046758595253_62166400'
  app/views/request/show.html.erb:68:in `_app_views_request_show_html_erb___1752266046758595253_62166400'
  app/controllers/request_controller.rb:133:in `block (3 levels) in show'
  app/controllers/request_controller.rb:132:in `block in show'
  lib/alaveteli_localization.rb:43:in `with_locale'
  app/controllers/request_controller.rb:83:in `show'
  app/controllers/application_controller.rb:120:in `record_memory'
  lib/strip_empty_sessions.rb:13:in `call'

Possible fix

In lib/alaveteli_external_command.rb, the find_program method returns exactly the argument it received, so the incoming mail handler fails when receiving an email in text/html format for instance, as it cannot find elinks. Patching the code like below seems to fix the problem for me, but maybe I just need to fix my config.

Happy to write a short PR if it is indeed a bug.

diff --git a/lib/alaveteli_external_command.rb b/lib/alaveteli_external_command.rb
index 6e4b3de..5433b65 100644
--- a/lib/alaveteli_external_command.rb
+++ b/lib/alaveteli_external_command.rb
@@ -68,7 +68,7 @@ module AlaveteliExternalCommand
         search_path = AlaveteliConfiguration::utility_search_path
         search_path.each do |d|
           program_path = File.join(d, program_name)
-          return program_name if File.file? program_path and File.executable? program_path
+          return program_path if File.file? program_path and File.executable? program_path
         end
         raise "Could not find #{program_name} in any of #{search_path.join(', ')}"
       end
lizconlan commented 5 years ago

Hi @laurentS - thanks for finding a bug! If you could submit that short PR, that would be great 🙂 (We'll get it merged and credit you in the next set of release notes!)

(I can only think we've not come across it before as in most cases elinks is found by the operating system without having to provide the path but that is not what the code is supposed to be doing; as you've found - it's helpfully figuring out the full path then discarding it 😞)