[BUG] Viewing changesets results in page not found

app/controllers/repositories_controller.rb

@@ -229,8 +229,13 @@ def find_repository
     @project = Project.find(params[:id])
     @repository = @project.repository
     (render_404; return false) unless @repository
-    @path = params[:path].join('/') unless params[:path].nil?
-    @path ||= ''
+    @path = ''
+    if params.has_key? :path
+      format_valid = params.has_key?(:format) && params[:format] != 'raw' && params[:format] != 'diff'
+
+      @path = params[:path]
+      @path += ".#{params[:format]}" if format_valid
+    end
     @rev = params[:rev].blank? ? @repository.default_branch : params[:rev].to_s.strip
     @rev_to = params[:rev_to]
 

app/helpers/application_helper.rb

@@ -385,7 +385,7 @@ def syntax_highlight(name, content)
   end
 
   def to_path_param(path)
-    path.to_s.split(%r{[/\\]}).select {|p| !p.blank?}
+    path.to_s
   end
 
 

app/models/repository/subversion.rb

@@ -40,7 +40,7 @@ def latest_changesets(path, rev, limit=10)
 
   # Returns a path relative to the url of the repository
   def relative_path(path)
-    path.gsub(Regexp.new("^\/?#{Regexp.escape(relative_url)}"), '')
+    path.gsub(Regexp.new("^\/?#{Regexp.escape(relative_url)}\/"), '')
   end
 
   def fetch_changesets

config/routes.rb

@@ -368,13 +368,14 @@
       match '/projects/:id/repository/statistics', :action => :stats
       match '/projects/:id/repository/committers', :action => :committers
       match '/projects/:id/repository/graph', :action => :graph
+      match '/projects/:id/repository/diff', :action => :diff
       match '/projects/:id/repository/revisions', :action => :revisions
       match '/projects/:id/repository/revisions.:format', :action => :revisions
       match '/projects/:id/repository/revisions/:rev', :action => :revision
       match '/projects/:id/repository/revisions/:rev/diff/*path(.:format)', :action => :diff
-      match '/projects/:id/repository/revisions/:rev/raw/*path', :action => :entry, :format => 'raw', :rev => /[a-z0-9\.\-_]+/
+      match '/projects/:id/repository/revisions/:rev/raw/*path', :action => :entry, :kind => 'raw', :rev => /[a-z0-9\.\-_]+/
       match '/projects/:id/repository/revisions/:rev/:action/*path', :rev => /[a-z0-9\.\-_]+/
-      match '/projects/:id/repository/raw/*path', :action => :entry, :format => 'raw'
+      match '/projects/:id/repository/raw/*path', :action => :entry, :kind => 'raw'
       # TODO: why the following route is required?
       match '/projects/:id/repository/entry/*path', :action => :entry
       match '/projects/:id/repository/:action/*path'

doc/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * `#1755` Migrate helper-tests for issues into specs for work package
 * `#1766` Fixed bug: Viewing diff of Work Package description results in error 500
+* `#1767` Fixed bug: Viewing changesets results in "page not found"
 * `#1789` Move validation to Work Package
 * `#1800` Add settings to change software name and URL and add additional footer content
 * `#1808` Add option to log user for each request

spec/routing/repositories_routing_spec.rb

@@ -31,30 +31,33 @@
     it { get("/projects/testproject/repository/revisions/2457").should route_to( :controller => 'repositories', :action => 'revision', :id => 'testproject', :rev => '2457')}
   end
 
-  pending describe "diff" do
-    it { get("/projects/testproject/repository/revisions/2457/diff").should             route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject', :rev => '2457')}
-    it { get("/projects/testproject/repository/revisions/2457/diff.diff").should        route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject', :rev => '2457', :format => 'diff')}
-    it { get("/projects/testproject/repository/diff/path/to/file.c").should             route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject', :path => %w[path to file.c])}
-    it { get("/projects/testproject/repository/revisions/2/diff/path/to/file.c").should route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject', :path => %w[path to file.c], :rev => '2')}
+  describe "diff" do
+    pending describe "unknown diff roots" do
+      it { get("/projects/testproject/repository/revisions/2457/diff").should             route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject', :rev => '2457')}
+      it { get("/projects/testproject/repository/revisions/2457/diff.diff").should        route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject', :rev => '2457', :format => 'diff')}
+    end
+    it { get("/projects/testproject/repository/diff").should route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject')}
+    it { get("/projects/testproject/repository/diff/path/to/file.c").should             route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject', :path => "path/to/file", :format => 'c')}
+    it { get("/projects/testproject/repository/revisions/2/diff/path/to/file.c").should route_to( :controller => 'repositories', :action => 'diff', :id => 'testproject', :path => "path/to/file.c", :rev => '2')}
   end
 
-  pending describe "browse" do
-    it { get("/projects/testproject/repository/browse/path/to/file.c").should route_to( :controller => 'repositories', :action => 'browse', :id => 'testproject', :path => %w[path to file.c])}
+  describe "browse" do
+    it { get("/projects/testproject/repository/browse/path/to/file.c").should route_to( :controller => 'repositories', :action => 'browse', :id => 'testproject', :path => "path/to/file", :format => 'c')}
   end
 
-  pending describe "entry" do
-    it { get("/projects/testproject/repository/entry/path/to/file.c").should route_to( :controller => 'repositories', :action => 'entry', :id => 'testproject', :path => %w[path to file.c])}
-    it { get("/projects/testproject/repository/revisions/2/entry/path/to/file.c").should route_to( :controller => 'repositories', :action => 'entry', :id => 'testproject', :path => %w[path to file.c], :rev => '2')}
-    it { get("/projects/testproject/repository/raw/path/to/file.c").should route_to( :controller => 'repositories', :action => 'entry', :id => 'testproject', :path => %w[path to file.c], :format => 'raw')}
-    it { get("/projects/testproject/repository/revisions/2/raw/path/to/file.c").should route_to( :controller => 'repositories', :action => 'entry', :id => 'testproject', :path => %w[path to file.c], :rev => '2', :format => 'raw')}
+  describe "entry" do
+    it { get("/projects/testproject/repository/entry/path/to/file.c").should route_to( :controller => 'repositories', :action => 'entry', :id => 'testproject', :path => "path/to/file", :format => 'c')}
+    it { get("/projects/testproject/repository/revisions/2/entry/path/to/file.c").should route_to( :controller => 'repositories', :action => 'entry', :id => 'testproject', :path => "path/to/file", :rev => '2', :format => 'c')}
+    it { get("/projects/testproject/repository/raw/path/to/file.c").should route_to( :controller => 'repositories', :action => 'entry', :id => 'testproject', :path => "path/to/file", :format => 'c', :kind => 'raw')}
+    it { get("/projects/testproject/repository/revisions/2/raw/path/to/file.c").should route_to( :controller => 'repositories', :action => 'entry', :id => 'testproject', :path => "path/to/file", :rev => '2', :format => 'c', :kind => 'raw')}
   end
 
-  pending describe "annotate" do
-    it { get("/projects/testproject/repository/annotate/path/to/file.c").should route_to( :controller => 'repositories', :action => 'annotate', :id => 'testproject', :path => %w[path to file.c])}
+  describe "annotate" do
+    it { get("/projects/testproject/repository/annotate/path/to/file.c").should route_to( :controller => 'repositories', :action => 'annotate', :id => 'testproject', :path => "path/to/file", :format => 'c')}
   end
 
-  pending describe "changes" do
-    it { get("/projects/testproject/repository/changes/path/to/file.c").should route_to( :controller => 'repositories', :action => 'changes', :id => 'testproject', :path => %w[path to file.c])}
+  describe "changes" do
+    it { get("/projects/testproject/repository/changes/path/to/file.c").should route_to( :controller => 'repositories', :action => 'changes', :id => 'testproject', :path => "path/to/file", :format => 'c')}
   end
 
   describe "stats" do

test/functional/repositories_filesystem_controller_test.rb

@@ -51,7 +51,7 @@ def test_browse_root
     end
 
     def test_show_no_extension
-      get :entry, :id => PRJ_ID, :path => ['test']
+      get :entry, :id => PRJ_ID, :path => 'test'
       assert_response :success
       assert_template 'entry'
       assert_tag :tag => 'th',
@@ -61,14 +61,14 @@ def test_show_no_extension
     end
 
     def test_entry_download_no_extension
-      get :entry, :id => PRJ_ID, :path => ['test'], :format => 'raw'
+      get :entry, :id => PRJ_ID, :path => 'test', :format => 'raw'
       assert_response :success
       assert_equal 'application/octet-stream', @response.content_type
     end
 
     def test_show_non_ascii_contents
       with_settings :repositories_encodings => 'UTF-8,EUC-JP' do
-        get :entry, :id => PRJ_ID, :path => ['japanese', 'euc-jp.txt']
+        get :entry, :id => PRJ_ID, :path => 'japanese/euc-jp.txt'
         assert_response :success
         assert_template 'entry'
         assert_tag :tag => 'th',
@@ -80,7 +80,7 @@ def test_show_non_ascii_contents
 
     def test_show_utf16
       with_settings :repositories_encodings => 'UTF-16' do
-        get :entry, :id => PRJ_ID, :path => ['japanese', 'utf-16.txt']
+        get :entry, :id => PRJ_ID, :path => 'japanese/utf-16.txt'
         assert_response :success
 
         assert_select "tr" do
@@ -95,7 +95,7 @@ def test_show_utf16
 
     def test_show_text_file_should_send_if_too_big
       with_settings :file_max_size_displayed => 1 do
-        get :entry, :id => PRJ_ID, :path => ['japanese', 'big-file.txt']
+        get :entry, :id => PRJ_ID, :path => 'japanese/big-file.txt'
         assert_response :success
         assert_equal 'text/plain', @response.content_type
       end

test/functional/repositories_git_controller_test.rb

@@ -101,7 +101,7 @@ def test_browse_tag
     def test_browse_directory
       @repository.fetch_changesets
       @repository.reload
-      get :show, :id => 3, :path => ['images']
+      get :show, :id => 3, :path => 'images'
       assert_response :success
       assert_template 'show'
       assert_not_nil assigns(:entries)
@@ -117,7 +117,7 @@ def test_browse_directory
     def test_browse_at_given_revision
       @repository.fetch_changesets
       @repository.reload
-      get :show, :id => 3, :path => ['images'], :rev => '7234cb2750b63f47bff735edc50a1c0a433c2518'
+      get :show, :id => 3, :path => 'images', :rev => '7234cb2750b63f47bff735edc50a1c0a433c2518'
       assert_response :success
       assert_template 'show'
       assert_not_nil assigns(:entries)
@@ -127,14 +127,14 @@ def test_browse_at_given_revision
     end
 
     def test_changes
-      get :changes, :id => 3, :path => ['images', 'edit.png']
+      get :changes, :id => 3, :path => 'images/edit.png'
       assert_response :success
       assert_template 'changes'
       assert_tag :tag => 'h2', :content => 'edit.png'
     end
 
     def test_entry_show
-      get :entry, :id => 3, :path => ['sources', 'watchers_controller.rb']
+      get :entry, :id => 3, :path => 'sources/watchers_controller.rb'
       assert_response :success
       assert_template 'entry'
       # Line 19
@@ -145,14 +145,14 @@ def test_entry_show
     end
 
     def test_entry_download
-      get :entry, :id => 3, :path => ['sources', 'watchers_controller.rb'], :format => 'raw'
+      get :entry, :id => 3, :path => 'sources/watchers_controller.rb', :format => 'raw'
       assert_response :success
       # File content
       assert @response.body.include?('WITHOUT ANY WARRANTY')
     end
 
     def test_directory_entry
-      get :entry, :id => 3, :path => ['sources']
+      get :entry, :id => 3, :path => 'sources'
       assert_response :success
       assert_template 'show'
       assert_not_nil assigns(:entry)
@@ -191,7 +191,7 @@ def test_diff_two_revs
     end
 
     def test_annotate
-      get :annotate, :id => 3, :path => ['sources', 'watchers_controller.rb']
+      get :annotate, :id => 3, :path => 'sources/watchers_controller.rb'
       assert_response :success
       assert_template 'annotate'
       # Line 23, changeset 2f9c0091
@@ -204,14 +204,14 @@ def test_annotate
     def test_annotate_at_given_revision
       @repository.fetch_changesets
       @repository.reload
-      get :annotate, :id => 3, :rev => 'deff7', :path => ['sources', 'watchers_controller.rb']
+      get :annotate, :id => 3, :rev => 'deff7', :path => 'sources/watchers_controller.rb'
       assert_response :success
       assert_template 'annotate'
       assert_tag :tag => 'h2', :content => /@ deff712f/
     end
 
     def test_annotate_binary_file
-      get :annotate, :id => 3, :path => ['images', 'edit.png']
+      get :annotate, :id => 3, :path => 'images/edit.png'
       assert_response 500
       assert_tag :tag => 'p', :attributes => { :id => /errorExplanation/ },
                               :content => /cannot be annotated/

test/functional/repositories_subversion_controller_test.rb

@@ -68,7 +68,7 @@ def test_browse_root
     def test_browse_directory
       @repository.fetch_changesets
       @repository.reload
-      get :show, :id => PRJ_ID, :path => ['subversion_test']
+      get :show, :id => PRJ_ID, :path => 'subversion_test'
       assert_response :success
       assert_template 'show'
       assert_not_nil assigns(:entries)
@@ -82,7 +82,7 @@ def test_browse_directory
     def test_browse_at_given_revision
       @repository.fetch_changesets
       @repository.reload
-      get :show, :id => PRJ_ID, :path => ['subversion_test'], :rev => 4
+      get :show, :id => PRJ_ID, :path => 'subversion_test', :rev => 4
       assert_response :success
       assert_template 'show'
       assert_not_nil assigns(:entries)
@@ -92,7 +92,7 @@ def test_browse_at_given_revision
     def test_file_changes
       @repository.fetch_changesets
       @repository.reload
-      get :changes, :id => PRJ_ID, :path => ['subversion_test', 'folder', 'helloworld.rb' ]
+      get :changes, :id => PRJ_ID, :path => 'subversion_test/folder/helloworld.rb'
       assert_response :success
       assert_template 'changes'
 
@@ -114,7 +114,7 @@ def test_file_changes
     def test_directory_changes
       @repository.fetch_changesets
       @repository.reload
-      get :changes, :id => PRJ_ID, :path => ['subversion_test', 'folder' ]
+      get :changes, :id => PRJ_ID, :path => 'subversion_test/folder'
       assert_response :success
       assert_template 'changes'
 
@@ -126,7 +126,7 @@ def test_directory_changes
     def test_entry
       @repository.fetch_changesets
       @repository.reload
-      get :entry, :id => PRJ_ID, :path => ['subversion_test', 'helloworld.c']
+      get :entry, :id => PRJ_ID, :path => 'subversion_test/helloworld.c'
       assert_response :success
       assert_template 'entry'
     end
@@ -136,7 +136,7 @@ def test_entry_should_send_if_too_big
       @repository.reload
       # no files in the test repo is larger than 1KB...
       with_settings :file_max_size_displayed => 0 do
-        get :entry, :id => PRJ_ID, :path => ['subversion_test', 'helloworld.c']
+        get :entry, :id => PRJ_ID, :path => 'subversion_test/helloworld.c'
         assert_response :success
         assert_template nil
         assert_equal 'attachment; filename="helloworld.c"', @response.headers['Content-Disposition']
@@ -146,7 +146,7 @@ def test_entry_should_send_if_too_big
     def test_entry_at_given_revision
       @repository.fetch_changesets
       @repository.reload
-      get :entry, :id => PRJ_ID, :path => ['subversion_test', 'helloworld.rb'], :rev => 2
+      get :entry, :id => PRJ_ID, :path => 'subversion_test/helloworld.rb', :rev => 2
       assert_response :success
       assert_template 'entry'
       # this line was removed in r3 and file was moved in r6
@@ -157,15 +157,15 @@ def test_entry_at_given_revision
     def test_entry_not_found
       @repository.fetch_changesets
       @repository.reload
-      get :entry, :id => PRJ_ID, :path => ['subversion_test', 'zzz.c']
+      get :entry, :id => PRJ_ID, :path => 'subversion_test/zzz.c'
       assert_tag :tag => 'p', :attributes => { :id => /errorExplanation/ },
                                 :content => /The entry or revision was not found in the repository/
     end
 
     def test_entry_download
       @repository.fetch_changesets
       @repository.reload
-      get :entry, :id => PRJ_ID, :path => ['subversion_test', 'helloworld.c'], :format => 'raw'
+      get :entry, :id => PRJ_ID, :path => 'subversion_test/helloworld.c', :format => 'raw'
       assert_response :success
       assert_template nil
       assert_equal 'attachment; filename="helloworld.c"', @response.headers['Content-Disposition']
@@ -174,7 +174,7 @@ def test_entry_download
     def test_directory_entry
       @repository.fetch_changesets
       @repository.reload
-      get :entry, :id => PRJ_ID, :path => ['subversion_test', 'folder']
+      get :entry, :id => PRJ_ID, :path => 'subversion_test/folder'
       assert_response :success
       assert_template 'show'
       assert_not_nil assigns(:entry)
@@ -192,7 +192,7 @@ def test_revision
                  :child => { :tag => 'li',
                              # link to the entry at rev 2
                              :child => { :tag => 'a',
-                                         :attributes => {:href => '/projects/ecookbook/repository/revisions/2/raw/test/some/path/in/the/repo'},
+                                         :attributes => {:href => '/projects/ecookbook/repository/revisions/2/entry/test/some/path/in/the/repo'},
                                          :content => 'repo',
                                          # link to partial diff
                                          :sibling =>  { :tag => 'a',
@@ -239,7 +239,7 @@ def test_revision_with_repository_pointing_to_a_subdirectory
                  :child => { :tag => 'li',
                              # link to the entry at rev 2
                              :child => { :tag => 'a',
-                                         :attributes => {:href => '/projects/ecookbook/repository/revisions/2/raw/path/in/the/repo'},
+                                         :attributes => {:href => '/projects/ecookbook/repository/revisions/2/entry/path/in/the/repo'},
                                          :content => 'repo',
                                          # link to partial diff
                                          :sibling =>  { :tag => 'a',
@@ -262,7 +262,7 @@ def test_revision_diff
     def test_directory_diff
       @repository.fetch_changesets
       @repository.reload
-      get :diff, :id => PRJ_ID, :rev => 6, :rev_to => 2, :path => ['subversion_test', 'folder']
+      get :diff, :id => PRJ_ID, :rev => 6, :rev_to => 2, :path => 'subversion_test/folder'
       assert_response :success
       assert_template 'diff'
 
@@ -277,15 +277,15 @@ def test_directory_diff
     def test_annotate
       @repository.fetch_changesets
       @repository.reload
-      get :annotate, :id => PRJ_ID, :path => ['subversion_test', 'helloworld.c']
+      get :annotate, :id => PRJ_ID, :path => 'subversion_test/helloworld.c'
       assert_response :success
       assert_template 'annotate'
     end
 
     def test_annotate_at_given_revision
       @repository.fetch_changesets
       @repository.reload
-      get :annotate, :id => PRJ_ID, :rev => 8, :path => ['subversion_test', 'helloworld.c']
+      get :annotate, :id => PRJ_ID, :rev => 8, :path => 'subversion_test/helloworld.c'
       assert_response :success
       assert_template 'annotate'
       assert_tag :tag => 'h2', :content => /@ 8/