Project

General

Profile

Actions
Copy link

Defect #13644

closed

Diff - Internal Error

Added by Todd Hambley about 11 years ago. Updated over 10 years ago.

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

0%

Estimated time:
Resolution:
Fixed
Affected version:
2.3.0

Description

Received error 500 attempting to view diff. I attached the files being compared.

An ActionView::Template::Error occurred in repositories#diff:

  undefined method `ord' for nil:NilClass
  lib/redmine/unified_diff.rb:209:in `offsets'

Partial Trace:

  lib/redmine/unified_diff.rb:209:in `offsets'
  lib/redmine/unified_diff.rb:187:in `write_offsets'
  lib/redmine/unified_diff.rb:184:in `times'
  lib/redmine/unified_diff.rb:184:in `write_offsets'
  lib/redmine/unified_diff.rb:163:in `parse_line'
  lib/redmine/unified_diff.rb:85:in `add_line'
  lib/redmine/unified_diff.rb:33:in `initialize'
  lib/redmine/unified_diff.rb:31:in `each'
  lib/redmine/unified_diff.rb:31:in `initialize'
  app/views/common/_diff.html.erb:1:in `new'
  app/views/common/_diff.html.erb:1:in `_app_views_common__diff_html_erb__374540104_111303620'
  actionpack (3.2.13) lib/action_view/template.rb:145:in `send'
  actionpack (3.2.13) lib/action_view/template.rb:145:in `render'

Environment:

  Redmine version                          2.3.0.stable.11692
  Ruby version                             1.8.7 (i686-linux)
  Rails version                            3.2.13
  Environment                              production
  Database adapter                         MySQL

Files

Download all files
TL_NGRID(376).ASL (4.54 KB) TL_NGRID(376).ASL Current version of file Todd Hambley, 2013-03-29 19:35
TL_NGRID(374).ASL (4.06 KB) TL_NGRID(374).ASL Previous version of file Todd Hambley, 2013-03-29 19:35
unified_diff.rb.patch (885 Bytes) unified_diff.rb.patch lib/redmine/unified_diff (as of 2.3.0, trunk) Jongwook Choi, 2013-04-05 02:57
b.diff (145 Bytes) b.diff Toshi MARUYAMA, 2013-04-05 07:22
unified_diff.rb_r12041.diff (4.58 KB) unified_diff.rb_r12041.diff Jun NAITOH, 2013-07-27 23:48
Actions
Copy link
 #1

Updated by Ivan Cenov about 11 years ago

I confirm.

Environment:
  Redmine version                          2.3.0.stable
  Ruby version                             1.9.3 (i386-mingw32)
  Rails version                            3.2.13
  Environment                              production
  Database adapter                         Mysql2
Redmine plugins:
  clipboard_image_paste                    1.5
  projects_tree_view                       0.0.8
  redmine_apijs                            4.2.0
  redmine_code_review                      0.6.1
  redmine_didyoumean                       1.2.0
  redmine_favourite_projects               0.6
  redmine_information                      1.0.2
  redmine_issue_checklist                  2.0.5
  redmine_local_avatars                    0.1.1
  redmine_pastebin                         0.2.0
  redmine_plugin_views_revisions           0.0.1
  redmine_theme_changer                    0.1.0
  redmine_user_issues                      0.0.2
  redmine_wiki_gchart_formula              0.0.5
  redmine_xls_export                       0.2.1
  redmine_youtube                          0.0.1
  sidebar_hide                             0.0.2
  wiking                                   0.0.3
Actions
Copy link
 #2

Updated by Jongwook Choi about 11 years ago

This is a major issue, please fix it ASAP.

The commit that introduced this is r11549 (#12641). A mistake :(
In 'lib/redmine/unified_diff.rb', the following hunks should be fixed:

-        while line_left[starting].ord.between?(128, 191) && starting > 0
+        while starting > 0 && line_left[starting].ord.between?(128, 191)
 ...(omitted)...
-        while line_left[ending].ord.between?(128, 191) && ending > -1
+        while ending > -1 && line_left[ending].ord.between?(128, 191)

I've submitted the patch.

Actions
Copy link
 #3

Updated by Filou Centrinov about 11 years ago

It reminds me to #12641 (r11551). I suggested at first a the patch containing the lines with ".ord.between?(128, 191)" but recognized later a much more elegant solution and added a second patch that just sets the encoding. The 2nd patch hasn't been taken. Why not simply setting the encoding?

Actions
Copy link
 #4

Updated by Toshi MARUYAMA about 11 years ago

Filou Centrinov wrote:

Why not simply setting the encoding?

Because tests fail on Ruby 1.8.7.

Actions
Copy link
 #5

Updated by Toshi MARUYAMA about 11 years ago

I cannot reproduce on ruby 1.8.7 (2012-10-12 patchlevel 371) [x86_64-linux].


$ irb
1.8.7 :001 > s = "abc" 
 => "abc" 
1.8.7 :002 > s[-1].ord
 => 99 
1.8.7 :003 > s = "日本語" 
 => "\346\227\245\346\234\254\350\252\236" 
1.8.7 :004 > s[-1].ord
 => 158

Actions
Copy link
 #6

Updated by Toshi MARUYAMA about 11 years ago

  • File b.diff b.diff added
  • Status changed from New to Confirmed

I got it.

Actions
Copy link
 #7

Updated by Etienne Massip about 11 years ago

  • Target version set to Candidate for next minor release
Actions
Copy link
 #8

Updated by Toshi MARUYAMA about 11 years ago

  • Target version changed from Candidate for next minor release to 2.3.1
Actions
Copy link
 #9

Updated by Toshi MARUYAMA about 11 years ago

  • Status changed from Confirmed to Closed
  • Resolution set to Fixed

Committed in trunk and 2.3-stable.

Actions
Copy link
 #10

Updated by Jun NAITOH over 10 years ago

"ending" is always negative value.
I think that I should correct it as follows.

--- unified_diff.rb_org 2013-07-25 20:57:37.000000000 +0900
+++ unified_diff.rb     2013-07-25 21:23:36.000000000 +0900
@@ -206,12 +206,12 @@
         end
         ending = -1
         while ending >= -(max - starting) && line_left[ending] == line_right[ending]
           ending -= 1
         end
-        if (! "".respond_to?(:force_encoding)) && ending > (-1 * line_left.size)
-          while line_left[ending].ord.between?(128, 191) && ending > -1
+        if (! "".respond_to?(:force_encoding))
+          while ending > (-1 * line_left.size) && line_left[ending].ord.between?(128, 191)
             ending -= 1
           end
         end
         unless starting == 0 && ending == -1
           [starting, ending]
Actions
Copy link
 #11

Updated by Toshi MARUYAMA over 10 years ago

Jun NAITOH wrote:

"ending" is always negative value.
I think that I should correct it as follows.

Could you give me examples or tests?

Actions
Copy link
 #12

Updated by Jun NAITOH over 10 years ago

Sorry, note-10 sample code has a problem.

I rewrote the code, and added test code.

Actions
Copy link
 #13

Updated by Toshi MARUYAMA over 10 years ago

Jun NAITOH wrote:

Sorry, note-10 sample code has a problem.

I rewrote the code, and added test code.

Thanks.
I have created new issue #14562.

Actions
Copy link

Also available in: Atom PDF