Refactor logic for determining per page upload location. Resolves #1830 (#1831)

* Refactor logic for determining per page upload location. Add tests.

* Add per page upload test for  context

lib/gollum/app.rb

@@ -241,22 +241,9 @@ module Precious
           tempfile = params[:file][:tempfile]
         end
         halt 500 unless tempfile.is_a? Tempfile
+        
+        dir = wiki.per_page_uploads ? find_per_page_upload_subdir(request.referer, request.host_with_port, wiki.base_path) : 'uploads'
 
-        if wiki.per_page_uploads
-          dir = request.referer.match(/^https?:\/\/#{request.host_with_port}\/(.*)/)[1]          
-          # remove base path if it is set
-          dir.sub!(/^#{wiki.base_path}/, '') if wiki.base_path
-          # remove base_url and gollum/* subpath if necessary
-          dir.sub!(/^\/gollum\/[-\w]+\//, '')
-          # remove file extension
-          dir.sub!(/#{::File.extname(dir)}$/, '')
-          # revert escaped whitespaces
-          dir.gsub!(/%20/, ' ')
-          dir = ::File.join('uploads', dir)
-        else
-          # store all uploads together
-          dir = 'uploads'
-        end
         halt 500 if dir.include?('..')
         halt 500 unless Pathname(dir).relative?
 

lib/gollum/helpers.rb

@@ -5,6 +5,20 @@ module Precious
   module Helpers
 
     EMOJI_PATHNAME = Pathname.new(Gemojione.images_path).freeze
+    
+    def find_per_page_upload_subdir(referer, host_with_port, base_path)
+      base = base_path ? remove_leading_and_trailing_slashes(base_path) : ''
+      dir = referer.match(/^https?:\/\/#{host_with_port}\/#{base}\/?(.*)/)[1]
+      
+      # remove gollum/* subpath if necessary
+      dir.sub!(/^gollum\/[-\w]+\//, '')
+      # remove file extension
+      dir.sub!(/#{::File.extname(dir)}$/, '')
+      # revert escaped whitespaces
+      dir.gsub!(/%20/, ' ')
+      
+      return ::File.join('uploads', dir)
+    end
 
     def sanitize_empty_params(param)
       [nil, ''].include?(param) ? nil : CGI.unescape(param)
@@ -14,6 +28,10 @@ module Precious
       # Check if name already has a format extension, and if so, strip it.
       Gollum::Page.valid_extension?(name) ? Gollum::Page.strip_filename(name) : name
     end
+    
+    def remove_leading_and_trailing_slashes(str)
+      str.sub(%r{^(/+)}, '').sub(%r{/+$}, '')
+    end
 
     # Remove all slashes from the start of string.
     # Remove all double slashes

test/test_app.rb

@@ -505,7 +505,21 @@ EOF
     assert_equal "abc", file.raw_data
     Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})
   end
+  
+  test 'upload a file with mode page from the edit page (drag and drop)' do
+    temp_upload_file = Tempfile.new(['upload', '.file']) << "abc\r"
+    temp_upload_file.close
+    Precious::App.set(:wiki_options, {allow_uploads: true, per_page_uploads: true})
+    post "/gollum/upload_file", {:file => Rack::Test::UploadedFile.new(::File.open(temp_upload_file))}, {'HTTP_REFERER' => 'http://localhost:4567/gollum/edit/foo/Bar.md', 'HTTP_HOST' => 'localhost:4567'}
 
+    assert_equal 302, last_response.status # redirect is expected
+    @wiki.clear_cache
+    # Find the file in a page-specific subdir (here: foo/Bar), based on referer
+    file = @wiki.file("uploads/foo/Bar/#{::File.basename(temp_upload_file.path)}")
+    assert_equal "abc\r", file.raw_data
+    Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})    
+  end
+  
   test "upload a file with https referer" do
     temp_upload_file = Tempfile.new(['https_upload', '.file']) << 'abc'
     temp_upload_file.close
@@ -520,7 +534,6 @@ EOF
     Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})
   end
 
-
   test "guard against uploading an existing file" do
     temp_upload_file = Tempfile.new(['upload', '.file']) << 'abc'
     temp_upload_file.close
@@ -1053,7 +1066,21 @@ context 'Frontend with base path' do
     assert last_response.ok?
     assert_equal '/wiki/gollum/history/Bilbo-Baggins.md', last_request.fullpath
   end
+  
+  test 'upload a file with mode page from the edit page (drag and drop)' do
+    temp_upload_file = Tempfile.new(['upload', '.file']) << "abc\r"
+    temp_upload_file.close
+    Precious::App.set(:wiki_options, {allow_uploads: true, per_page_uploads: true})
+    post "/wiki/gollum/upload_file", {:file => Rack::Test::UploadedFile.new(::File.open(temp_upload_file))}, {'HTTP_REFERER' => 'http://localhost:4567/wiki/gollum/edit/foo/Bar.md', 'HTTP_HOST' => 'localhost:4567'}
 
+    assert_equal 302, last_response.status # redirect is expected
+    @wiki.clear_cache
+    # Find the file in a page-specific subdir (here: foo/Bar), based on referer
+    file = @wiki.file("uploads/foo/Bar/#{::File.basename(temp_upload_file.path)}")
+    assert_equal "abc\r", file.raw_data
+    Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})    
+  end
+  
   def app
     Precious::MapGollum.new(@base_path)
   end

test/test_app_helpers.rb

@@ -0,0 +1,32 @@
+require File.expand_path(File.join(File.dirname(__FILE__), "helper"))
+
+context 'Precious::Helpers' do
+  include Precious::Helpers
+  
+  test 'remove trailing and leading slashes' do
+    ['/wiki', '/wiki/', 'wiki/', '//wiki//'].each do |param|
+      assert_equal 'wiki', remove_leading_and_trailing_slashes(param)
+    end
+    assert_equal 'wi/ki', remove_leading_and_trailing_slashes('/wi/ki/')
+    assert_equal '', remove_leading_and_trailing_slashes('/')
+  end
+  
+  test 'per page upload location helper' do
+    # https referer with and without base path
+    host_with_port = 'localhost:4567'
+    assert_equal 'uploads/Home', find_per_page_upload_subdir('https://localhost:4567/Home.md', host_with_port, nil)
+    assert_equal 'uploads/Home', find_per_page_upload_subdir('https://localhost:4567/wiki/Home.md', host_with_port, '/wiki')
+    
+    # http referer with and without base path
+    assert_equal 'uploads/Home', find_per_page_upload_subdir('http://localhost:4567/Home.md', host_with_port,  nil)
+    assert_equal 'uploads/Home', find_per_page_upload_subdir('http://localhost:4567/wiki/Home.md', host_with_port, '/wiki')
+    
+    # edit page referer with and without base path
+    assert_equal 'uploads/foo/Home', find_per_page_upload_subdir('http://localhost:4567/gollum/edit/foo/Home.md', host_with_port, nil)
+    assert_equal 'uploads/foo/Home', find_per_page_upload_subdir('http://localhost:4567/wiki/gollum/edit/foo/Home.md', host_with_port, '/wiki')
+    
+    # referer with base path with slashes in the wrong place
+    assert_equal 'uploads/Home', find_per_page_upload_subdir('http://localhost:4567/wiki/Home.md', host_with_port, '/wiki/')
+    assert_equal 'uploads/Home', find_per_page_upload_subdir('http://localhost:4567/wiki/Home.md', host_with_port, 'wiki')    
+  end
+end