mirror of
https://github.com/neocities/neocities.git
synced 2025-08-21 16:40:53 +02:00
fix for % in filenames
This commit is contained in:
parent
690f2f3c80
commit
a270d266c5
4 changed files with 70 additions and 2 deletions
|
@ -113,7 +113,6 @@ post '/api/upload' do
|
||||||
end
|
end
|
||||||
|
|
||||||
files.each do |file|
|
files.each do |file|
|
||||||
file[:filename] = Rack::Utils.unescape file[:filename]
|
|
||||||
if !current_site.okay_to_upload?(file)
|
if !current_site.okay_to_upload?(file)
|
||||||
api_error 400, 'invalid_file_type', "#{file[:filename]} is not an allowed file type for free sites, supporter required"
|
api_error 400, 'invalid_file_type', "#{file[:filename]} is not an allowed file type for free sites, supporter required"
|
||||||
end
|
end
|
||||||
|
|
|
@ -1229,7 +1229,8 @@ class Site < Sequel::Model
|
||||||
|
|
||||||
# https://practicingruby.com/articles/implementing-an-http-file-server?u=dc2ab0f9bb
|
# https://practicingruby.com/articles/implementing-an-http-file-server?u=dc2ab0f9bb
|
||||||
def scrubbed_path(path='')
|
def scrubbed_path(path='')
|
||||||
path ||= ''
|
path = path.to_s
|
||||||
|
|
||||||
clean = []
|
clean = []
|
||||||
|
|
||||||
parts = path.to_s.split '/'
|
parts = path.to_s.split '/'
|
||||||
|
|
|
@ -347,6 +347,31 @@ describe 'api' do
|
||||||
_(site_file_exists?('te[s]t.jpg')).must_equal true
|
_(site_file_exists?('te[s]t.jpg')).must_equal true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'succeeds with percent character in filename' do
|
||||||
|
create_site
|
||||||
|
@site.generate_api_key!
|
||||||
|
header 'Authorization', "Bearer #{@site.api_key}"
|
||||||
|
|
||||||
|
test_filenames = [
|
||||||
|
'100% awesome.jpg',
|
||||||
|
'dsfds/50%off.png',
|
||||||
|
'50% sale.txt',
|
||||||
|
'discount%special.png'
|
||||||
|
]
|
||||||
|
|
||||||
|
test_filenames.each do |filename|
|
||||||
|
post '/api/upload', filename => Rack::Test::UploadedFile.new('./tests/files/test.jpg', 'image/jpeg')
|
||||||
|
_(res[:result]).must_equal 'success'
|
||||||
|
_(site_file_exists?(filename)).must_equal true
|
||||||
|
|
||||||
|
# Verify the filename was stored literally, not URL-decoded
|
||||||
|
@site.reload # Reload to get fresh site_files
|
||||||
|
site_file = @site.site_files.find { |f| f.path == filename }
|
||||||
|
_(site_file).wont_be_nil
|
||||||
|
_(site_file.path).must_equal filename # Should be exactly as uploaded
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
it 'succeeds with valid user session' do
|
it 'succeeds with valid user session' do
|
||||||
create_site
|
create_site
|
||||||
post '/api/upload',
|
post '/api/upload',
|
||||||
|
|
|
@ -143,6 +143,49 @@ describe Site do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'scrubbed_path' do
|
||||||
|
it 'preserves literal percent characters without URL decoding' do
|
||||||
|
site = Fabricate :site
|
||||||
|
|
||||||
|
test_paths = [
|
||||||
|
'100% awesome.jpg',
|
||||||
|
'derpking/70%off.png',
|
||||||
|
'50% sale.txt',
|
||||||
|
'discount%special.png',
|
||||||
|
'garfield is 100% sexy.jpg',
|
||||||
|
'path/with/100%valid.txt'
|
||||||
|
]
|
||||||
|
|
||||||
|
test_paths.each do |path|
|
||||||
|
scrubbed = site.scrubbed_path(path)
|
||||||
|
_(scrubbed).must_equal path # Should be exactly the same - no URL decoding
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'still handles path traversal and other security issues' do
|
||||||
|
site = Fabricate :site
|
||||||
|
|
||||||
|
# Should still block path traversal
|
||||||
|
_(site.scrubbed_path('../../../etc/passwd')).must_equal 'etc/passwd'
|
||||||
|
_(site.scrubbed_path('../../test')).must_equal 'test'
|
||||||
|
|
||||||
|
# Should still remove empty components and dots
|
||||||
|
_(site.scrubbed_path('/./test/./file.txt')).must_equal 'test/file.txt'
|
||||||
|
_(site.scrubbed_path('test//file.txt')).must_equal 'test/file.txt'
|
||||||
|
|
||||||
|
# But percent characters should be preserved
|
||||||
|
_(site.scrubbed_path('test/70%off.png')).must_equal 'test/70%off.png'
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'raises error for control characters' do
|
||||||
|
site = Fabricate :site
|
||||||
|
|
||||||
|
# Should still raise error for control characters (below ASCII 32)
|
||||||
|
_(proc { site.scrubbed_path("test\x00file.txt") }).must_raise ArgumentError
|
||||||
|
_(proc { site.scrubbed_path("test\x1Ffile.txt") }).must_raise ArgumentError
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe 'custom_max_space' do
|
describe 'custom_max_space' do
|
||||||
it 'should use the custom max space if it is more' do
|
it 'should use the custom max space if it is more' do
|
||||||
site = Fabricate :site
|
site = Fabricate :site
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue