Project

General

Profile

Actions

Patch #9484

closed

Limit SCM annotate to text files under the maximum file size for viewing

Added by Antonio García-Domínguez over 12 years ago. Updated over 12 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
SCM
Target version:
Start date:
2011-10-28
Due date:
% Done:

0%

Estimated time:

Description

After recently upgrading to Redmine 1.2.1 + patch #4905 and moving from Mongrel 1.1.5 to Passenger 3.0.9 (using Ruby 1.8.7 as packaged in Ubuntu's ruby1.8 1.8.7.249-2 package, for amd64), I started getting rogue processes which used up 100% CPU every now and then. Killing these rogue processes using "kill -SIGABRT pid" produced entries like these:

Processing RepositoriesController#annotate (for 10.182.221.1 at 2011-10-28 01:51:08) [GET]
  Parameters: {"action"=>"annotate", "id"=>"sources-fm", "path"=>["trunk", "xmleye-1.2.3_svn473-bpelunit_results.tar.gz"], "controller"=>"repositories"}
Rendering template within layouts/base
Rendering repositories/annotate

ActionView::TemplateError (SIGTERM) on line #17 of app/views/repositories/annotate.rhtml:
14: <table class="filecontent annotate syntaxhl">
15:   <tbody>
16:     <% line_num = 1 %>
17:     <% syntax_highlight(@path, to_utf8(@annotate.content)).each_line do |line| %>
18:     <% revision = @annotate.revisions[line_num-1] %>
19:     <tr class="bloc-<%= revision.nil? ? 0 : colors[revision.identifier || revision.revision] %>">
20:       <th class="line-num" id="L<%= line_num %>"><a href="#L<%= line_num %>"><%= line_num %></a></th>

    lib/redmine/codeset_util.rb:21:in `replace_invalid_utf8'
    app/helpers/repositories_helper.rb:146:in `to_utf8_internal'
    app/helpers/repositories_helper.rb:121:in `to_utf8'
    app/views/repositories/annotate.rhtml:17:in `_run_rhtml_app47views47repositories47annotate46rhtml'
    passenger (3.0.9) lib/phusion_passenger/rack/request_handler.rb:96:in `process_request'
    passenger (3.0.9) lib/phusion_passenger/abstract_request_handler.rb:513:in `accept_and_process_next_request'
    passenger (3.0.9) lib/phusion_passenger/abstract_request_handler.rb:274:in `main_loop'
    passenger (3.0.9) lib/phusion_passenger/classic_rails/application_spawner.rb:321:in `start_request_handler'
    passenger (3.0.9) lib/phusion_passenger/classic_rails/application_spawner.rb:275:in `send'
    passenger (3.0.9) lib/phusion_passenger/classic_rails/application_spawner.rb:275:in `handle_spawn_application'
    passenger (3.0.9) lib/phusion_passenger/utils.rb:479:in `safe_fork'
    passenger (3.0.9) lib/phusion_passenger/classic_rails/application_spawner.rb:270:in `handle_spawn_application'
    passenger (3.0.9) lib/phusion_passenger/abstract_server.rb:357:in `__send__'
    passenger (3.0.9) lib/phusion_passenger/abstract_server.rb:357:in `server_main_loop'
    passenger (3.0.9) lib/phusion_passenger/abstract_server.rb:206:in `start_synchronously'
    passenger (3.0.9) lib/phusion_passenger/abstract_server.rb:180:in `start'
    passenger (3.0.9) lib/phusion_passenger/classic_rails/application_spawner.rb:149:in `start'
    passenger (3.0.9) lib/phusion_passenger/spawn_manager.rb:219:in `spawn_rails_application'
    passenger (3.0.9) lib/phusion_passenger/abstract_server_collection.rb:132:in `lookup_or_add'
    passenger (3.0.9) lib/phusion_passenger/spawn_manager.rb:214:in `spawn_rails_application'
    passenger (3.0.9) lib/phusion_passenger/abstract_server_collection.rb:82:in `synchronize'
    passenger (3.0.9) lib/phusion_passenger/abstract_server_collection.rb:79:in `synchronize'
    passenger (3.0.9) lib/phusion_passenger/spawn_manager.rb:213:in `spawn_rails_application'
    passenger (3.0.9) lib/phusion_passenger/spawn_manager.rb:132:in `spawn_application'
    passenger (3.0.9) lib/phusion_passenger/spawn_manager.rb:275:in `handle_spawn_application'
    passenger (3.0.9) lib/phusion_passenger/abstract_server.rb:357:in `__send__'
    passenger (3.0.9) lib/phusion_passenger/abstract_server.rb:357:in `server_main_loop'
    passenger (3.0.9) lib/phusion_passenger/abstract_server.rb:206:in `start_synchronously'
    passenger (3.0.9) helper-scripts/passenger-spawn-server:99

As you can see, it was trying to show the output of calling 'svn annotate' on a medium (~18MB) tar.gz file, while trying to interpret its contents as UTF-8 text.

We already have a setting for limiting the maximum size of the text files which can be viewed (instead of forcing the download) using 'View'. I think we should use the same limit for annotating the files, and also prevent Redmine from annotating binary files (using the same heuristics as in 'View').

I have attached a patch which modifies 'annotate' so it checks that the file is really a text file under the specified size limit before trying to annotate it. Otherwise, it displays a new error message explaining why it could not be annotated.


Files

0001-Fix-for-expensive-annotations.patch (4.1 KB) 0001-Fix-for-expensive-annotations.patch Patch which limits 'annotate' to text files under the global size limit for viewing files directly Antonio García-Domínguez, 2011-10-28 02:07
revised-9484.patch (4.79 KB) revised-9484.patch Antonio García-Domínguez, 2011-10-30 16:47
Actions

Also available in: Atom PDF