Defect #30850

Unified diff link broken on specific file/revision diff view

Added by Anthony Mallet about 1 month ago. Updated 8 days ago.

Status:ResolvedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:SCM
Target version:4.0.3
Resolution: Affected version:4.0.1

Description

Clicking on the "Unified diff" linkt 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.

Associated revisions

Revision 17962
Added by Jean-Philippe Lang 8 days ago

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

History

#1 Updated by Go MAEDA about 1 month ago

  • Status changed from New to Confirmed

#2 Updated by Go MAEDA about 1 month ago

  • Target version set to 4.0.2

#3 Updated by Jean-Philippe Lang 30 days ago

  • Target version changed from 4.0.2 to 4.0.3

#4 Updated by Mizuki ISHIKAWA 16 days 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 days 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 days 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?

Also available in: Atom PDF