Redmine - Patch #9484
Limit SCM annotate to text files under the maximum file size for viewing
2011-10-28 02:07 - Antonio García-Domínguez

ステータス:

Closed

開始日:

優先度:

Normal

期日:

担当者:
カテゴリ:

SCM

対象バージョン:1.3.0

2011-10-28

進捗率:

0%

予定工数:

0.00時間説明
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 aspackaged 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"],er"=>"repositories"}
Rendering template within layouts/baseRendering 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 interpretits contents as UTF-8 text.

2024-10-11

1/2"controll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.
関係しているリビジョン
リビジョン 7728 - 2011-11-05 04:47 - Toshi MARUYAMA
do not annotate text files which exceed the size limit for viewing (#9484)
Contributed by Antonio García-Domínguez.
リビジョン 7729 - 2011-11-05 04:47 - Toshi MARUYAMA
update i18n locale yamls (#9484)

履歴#1 - 2011-10-28 03:18 - Toshi MARUYAMAGreat work!
Could you add a test such as source:tags/1.2.1/test/functional/repositories̲filesystem̲controller̲test.rb#L115 ?
#2 - 2011-10-30 00:13 - Antonio García-Domínguez
Toshi MARUYAMA wrote:
Great work!
Could you add a test such as source:tags/1.2.1/test/functional/repositories̲filesystem̲controller̲test.rb#L115 ?

I'm glad you liked the patch :-). Sorry, I forgot to watch the issue and didn't see your message until now. I'll write it tomorrow: it's midnight rightnow in Spain. Basically, I just need to test that the proper error is shown when trying to annotate a file that is too big, right?
#3 - 2011-10-30 01:22 - Toshi MARUYAMA
Yes. Please see source:tags/1.2.1/test/functional/repositories̲git̲controller̲test.rb#L306#4 - 2011-10-30 16:47 - Antonio García-Domínguez- ファイル revised-9484.patch を追加
I have revised the patch with a new test case for the Subversion test suite. I had to change the new code a bit as well, as it was breaking one of thefunctional tests for Git repositories. It seems that with the old patch, a different error message would have been shown when trying to annotate
binary files.
I have tried this one with 'rake test RAILS̲ENV=test' using a SQLite database: all the tests seem to pass. I haven't run the tests that require an LDAP
server, though.
#5 - 2011-11-05 06:27 - Toshi MARUYAMA- ステータス を New から Closed に変更- 対象バージョン を 1.3.0 にセット
Committed in trunk r7728 and r7729, thanks.

ファイル
0001-Fix-for-expensive-annotations.patch

4.1 KB

2011-10-28

Antonio García-Domínguez

revised-9484.patch

4.79 KB

2011-10-30

Antonio García-Domínguez

2024-10-11

2/2