Defect #30850

Unified diff link broken on specific file/revision diff view

Added by Anthony Mallet 9 months ago. Updated 5 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:SCM
Target version:4.0.4
Resolution:Fixed Affected version:4.0.1

Description

Clicking on the "Unified diff" link at the bottom of a diff page of a specific revision doesn't work anymore.

For a git revision, it simply produces a "Page not found" error.

Url are like 'https://.../projects/:project_id/repository/:repository_id/revisions/:rev/diff.diff, the last component is interpreted as a (/*path) according to routes.rb (lines 250 and 258)

This was fixed in issue #11325 (revision 9909), but was broken in 4.0.1 again.


Related issues

Related to Redmine - Defect #31141: SCM: error when *.yml and *.txt show Closed

Associated revisions

Revision 17962
Added by Jean-Philippe Lang 8 months ago

Unified diff link broken on specific file/revision diff view (#30850).

Revision 18206
Added by Toshi MARUYAMA 6 months ago

scm: early return in diff if diff is nil as same as show, changes, and etc. (#31141, #30850)

Contributed by Mizuki ISHIKAWA.

Revision 18207
Added by Toshi MARUYAMA 6 months ago

add *.yaml and *.txt to RoutingRepositoriesTest (#31141, #30850)

Revision 18211
Added by Toshi MARUYAMA 6 months ago

scm: fix error when *.yml and *.txt show (#31141, #30850)

Contributed by Mizuki ISHIKAWA.

Revision 18242
Added by Jean-Philippe Lang 5 months ago

Merged r17962, r18206, r18207 and r18211 to 4.0-stable (#30850).

History

#1 Updated by Go MAEDA 9 months ago

  • Status changed from New to Confirmed

#2 Updated by Go MAEDA 9 months ago

  • Target version set to 4.0.2

#3 Updated by Jean-Philippe Lang 9 months ago

  • Target version changed from 4.0.2 to 4.0.3

#4 Updated by Mizuki ISHIKAWA 8 months ago

By changing to the following, the diff format is allowed.

diff --git a/config/routes.rb b/config/routes.rb
index 0344982085..07287f208d 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -250,7 +250,7 @@ Rails.application.routes.draw do
     get "projects/:id/repository/:repository_id/revisions/:rev/#{action}(/*path)",
         :controller => 'repositories',
         :action => action,
-        :format => 'html',
+        :format => (action == 'diff' ? /(html|diff)/ : 'html'),
         :constraints => {:rev => /[a-z0-9\.\-_]+/, :path => /.*/}
   end

@@ -258,7 +258,7 @@ Rails.application.routes.draw do
     get "projects/:id/repository/:repository_id/#{action}(/*path)",
         :controller => 'repositories',
         :action => action,
-        :format => 'html',
+        :format => (action == 'diff' ? /(html|diff)/ : 'html'),
         :constraints => {:path => /.*/}
   end

#5 Updated by Jean-Philippe Lang 8 months ago

Mizuki ISHIKAWA wrote:

By changing to the following, the diff format is allowed.

[...]

This change doesn't work when trying to get the diff of a single file of a revision, eg:
/projects/projet-a/repository/svn2/revisions/10/diff/subversion_test/folder/subfolder/journals_controller.rb.diff => 404 not found

#6 Updated by Jean-Philippe Lang 8 months ago

  • Status changed from Confirmed to Resolved

Fix committed in r17962 by restoring :format => false just like it was in 3.4.
This was changed when upgrading to Rails 5 because :format => false was no longer working a sexpected, I suspect that a Rails regression was fixed lately.
I've also added some integration tests to make sure that browsing diffs no longer breaks.

Just in case, could anyone confirm this is fixed?

#7 Updated by Jean-Philippe Lang 8 months ago

  • Status changed from Resolved to Closed
  • Assignee set to Jean-Philippe Lang
  • Resolution set to Fixed

#8 Updated by Go MAEDA 6 months ago

  • Related to Defect #31141: SCM: error when *.yml and *.txt show added

#9 Updated by Go MAEDA 6 months ago

  • Status changed from Closed to Reopened
  • Target version changed from 4.0.3 to 4.0.4

The fix in r17962 caused another problem #31141.

And there is another problem regarding this issue. Although the target version is 4.0.3, the change has not merged to 4.0-stable branch. I am reopening this issue and change the target version to 4.0.4.

#10 Updated by Toshi MARUYAMA 6 months ago

  • Description updated (diff)

#11 Updated by Jean-Philippe Lang 5 months ago

  • Status changed from Reopened to Closed

Merged to 4.0-stable, thanks for digging into this!

Also available in: Atom PDF