diff --git a/app/site_files.rb b/app/site_files.rb
index ef0b8bb2..fa429c50 100644
--- a/app/site_files.rb
+++ b/app/site_files.rb
@@ -130,7 +130,9 @@ post '/site_files/upload' do
end
end
- file[:filename] = "#{dir_name.force_encoding('UTF-8')}/#{site.scrubbed_path file[:filename].force_encoding('UTF-8')}"
+ file_base_name = site.scrubbed_path file[:filename].force_encoding('UTF-8')
+
+ file[:filename] = "#{dir_name.force_encoding('UTF-8')}/#{file_base_name}"
if current_site.file_size_too_large? file[:tempfile].size
file_upload_response "#{Rack::Utils.escape_html file[:filename]} is too large, upload cancelled."
@@ -138,6 +140,12 @@ post '/site_files/upload' do
if !site.okay_to_upload? file
file_upload_response %{#{Rack::Utils.escape_html file[:filename]}: file type (or content in file) is only supported by supporter accounts. Why We Do This}
end
+ if SiteFile.path_too_long? file[:filename]
+ file_upload_response "#{Rack::Utils.escape_html file[:filename]}: path is too long, upload cancelled."
+ end
+ if SiteFile.name_too_long? file_base_name
+ file_upload_response "#{Rack::Utils.escape_html file[:filename]}: file name is too long, upload cancelled."
+ end
end
uploaded_size = params[:files].collect {|f| f[:tempfile].size}.inject{|sum,x| sum + x }
diff --git a/models/site_file.rb b/models/site_file.rb
index 40876358..52305766 100644
--- a/models/site_file.rb
+++ b/models/site_file.rb
@@ -5,10 +5,22 @@ require 'sanitize'
class SiteFile < Sequel::Model
CLASSIFIER_LIMIT = 1_000_000
CLASSIFIER_WORD_LIMIT = 25
+ FILE_PATH_CHARACTER_LIMIT = 300
+ FILE_NAME_CHARACTER_LIMIT = 100
unrestrict_primary_key
plugin :update_primary_key
many_to_one :site
+ def self.path_too_long?(filename)
+ return true if filename.length > FILE_PATH_CHARACTER_LIMIT
+ false
+ end
+
+ def self.name_too_long?(filename)
+ return true if filename.length > FILE_NAME_CHARACTER_LIMIT
+ false
+ end
+
def before_destroy
if is_directory
site.site_files_dataset.where(path: /^#{Regexp.quote path}\//, is_directory: true).all.each do |site_file|
diff --git a/tests/site_file_tests.rb b/tests/site_file_tests.rb
index 1c9b7547..24643d17 100644
--- a/tests/site_file_tests.rb
+++ b/tests/site_file_tests.rb
@@ -270,6 +270,28 @@ describe 'site_files' do
_(File.exists?(@site.files_path('cache.manifest'))).must_equal true
end
+ it 'fails with filename greater than limit' do
+ file_path = './tests/files' + (0...SiteFile::FILE_NAME_CHARACTER_LIMIT+1).map { ('a'..'z').to_a[rand(26)] }.join + '.html'
+ begin
+ File.open(file_path, "w") do |file|
+ file.write("derp")
+ end
+
+ upload 'files[]' => Rack::Test::UploadedFile.new(file_path, 'text/html')
+ _(last_response.body).must_match /name is too long/i
+ ensure
+ FileUtils.rm file_path
+ end
+ end
+
+ it 'fails with path greater than limit' do
+ upload(
+ 'dir' => (("a" * 50 + "/") * (SiteFile::FILE_PATH_CHARACTER_LIMIT / 50 - 1) + "a" * 50),
+ 'files[]' => Rack::Test::UploadedFile.new('./tests/files/test.jpg', 'image/jpeg')
+ )
+ _(last_response.body).must_match /path is too long/i
+ end
+
it 'works with otf fonts' do
upload 'files[]' => Rack::Test::UploadedFile.new('./tests/files/chunkfive.otf', 'application/vnd.ms-opentype')
_(File.exists?(@site.files_path('chunkfive.otf'))).must_equal true