From 4371374ba6d0eed7b54d33dc44a5ad2525dd6d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 29 Apr 2014 19:12:35 +0200 Subject: [PATCH] FEATURE: support for enabling all upload file types BUGFIX: authorized extensions is now case insensitive --- .../javascripts/discourse/lib/utilities.js | 50 +++++++++++++------ config/locales/server.en.yml | 2 +- config/site_settings.yml | 2 +- lib/file_helper.rb | 2 +- lib/validators/upload_validator.rb | 17 +++++-- spec/controllers/uploads_controller_spec.rb | 20 +++++++- 6 files changed, 70 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/utilities.js b/app/assets/javascripts/discourse/lib/utilities.js index 200b3bbb923..cc268521726 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js +++ b/app/assets/javascripts/discourse/lib/utilities.js @@ -7,9 +7,6 @@ **/ Discourse.Utilities = { - IMAGE_EXTENSIONS: [".png", ".jpg", ".jpeg", ".gif", ".bmp", ".tif", ".tiff"], - IS_AN_IMAGE_REGEXP: /\.(png|jpg|jpeg|gif|bmp|tif|tiff)$/i, - translateSize: function(size) { switch (size) { case 'tiny': return 20; @@ -180,9 +177,9 @@ Discourse.Utilities = { @returns true whenever the upload is valid **/ validateUploadedFile: function(file, type) { - // check that the uploaded file is authorized - if (!Discourse.Utilities.isAuthorizedUpload(file)) { + if (!Discourse.Utilities.authorizesAllExtensions() && + !Discourse.Utilities.isAuthorizedUpload(file)) { var extensions = Discourse.Utilities.authorizedExtensions(); bootbox.alert(I18n.t('post.errors.upload_not_authorized', { authorized_extensions: extensions })); return false; @@ -206,6 +203,16 @@ Discourse.Utilities = { return true; }, + + /** + Determine whether all file extensions are authorized. + + @method authorizesAllExtensions + **/ + authorizesAllExtensions: function() { + return Discourse.SiteSettings.authorized_extensions.indexOf("*") >= 0; + }, + /** Check the extension of the file against the list of authorized extensions @@ -213,9 +220,27 @@ Discourse.Utilities = { @param {File} file The file we want to upload **/ isAuthorizedUpload: function(file) { - var extensions = Discourse.SiteSettings.authorized_extensions; - var regexp = new RegExp("(" + extensions + ")$", "i"); - return file && file.name ? file.name.match(regexp) : false; + if (file && file.name) { + var extensions = _.chain(Discourse.SiteSettings.authorized_extensions.split("|")) + .reject(function(extension) { return extension.indexOf("*") >= 0; }) + .map(function(extension) { return (extension.indexOf(".") === 0 ? extension.substring(1) : extension).replace(".", "\\."); }) + .value(); + return new RegExp("\\.(" + extensions.join("|") + ")$", "i").test(file.name); + } + return false; + }, + + /** + List the authorized extension for display + + @method authorizedExtensions + **/ + authorizedExtensions: function() { + return _.chain(Discourse.SiteSettings.authorized_extensions.split("|")) + .reject(function(extension) { return extension.indexOf("*") >= 0; }) + .map(function(extension) { return extension.toLowerCase(); }) + .value() + .join(", "); }, /** @@ -239,7 +264,7 @@ Discourse.Utilities = { @param {String} path The path **/ isAnImage: function(path) { - return Discourse.Utilities.IS_AN_IMAGE_REGEXP.test(path); + return (/\.(png|jpg|jpeg|gif|bmp|tif|tiff)$/i).test(path); }, /** @@ -248,11 +273,8 @@ Discourse.Utilities = { @method allowsAttachments **/ allowsAttachments: function() { - return _.difference(Discourse.SiteSettings.authorized_extensions.split("|"), Discourse.Utilities.IMAGE_EXTENSIONS).length > 0; - }, - - authorizedExtensions: function() { - return Discourse.SiteSettings.authorized_extensions.replace(/\|/g, ", "); + return Discourse.Utilities.authorizesAllExtensions() || + (/(png|jpg|jpeg|gif|bmp|tif|tiff)/i).test(Discourse.SiteSettings.authorized_extensions); }, displayErrorForUpload: function(data) { diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4aa3c69ea43..cc9b61153b6 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -803,7 +803,7 @@ en: max_image_size_kb: "The maximum size of images we allow users to upload in kB - configure the limit in nginx (client_max_body_size) / apache or proxy as well." max_attachment_size_kb: "The maximum size of files we allow users to upload in kB - configure the limit in nginx (client_max_body_size) / apache or proxy as well." - authorized_extensions: "A pipe (|) separated list of file extensions allowed for upload" + authorized_extensions: "A pipe (|) separated list of file extensions allowed for upload ('*' for enabling all file types)" max_similar_results: "How many similar topics to show a user while they are composing a new topic" title_prettify: "Prevent common title typos and errors, including all caps, lowercase first character, multiple ! and ?, extra . at end, etc." diff --git a/config/site_settings.yml b/config/site_settings.yml index 2362f9b939a..0503e3573cc 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -284,7 +284,7 @@ files: default: 1024 authorized_extensions: client: true - default: '.jpg|.jpeg|.png|.gif' + default: 'jpg|jpeg|png|gif' refresh: true list: true crawl_images: diff --git a/lib/file_helper.rb b/lib/file_helper.rb index 0e99f92662f..4c6293a109f 100644 --- a/lib/file_helper.rb +++ b/lib/file_helper.rb @@ -31,7 +31,7 @@ class FileHelper end def self.images_regexp - @@images_regexp ||= /\.(#{images.to_a.join("|").gsub(".", "\.")})$/i + @@images_regexp ||= /\.(#{images.to_a.join("|")})$/i end end diff --git a/lib/validators/upload_validator.rb b/lib/validators/upload_validator.rb index 4ba0710d176..c778c64c8f4 100644 --- a/lib/validators/upload_validator.rb +++ b/lib/validators/upload_validator.rb @@ -47,30 +47,39 @@ class Validators::UploadValidator < ActiveModel::Validator .tr(" ", "") .split("|") .each do |extension| - authorized_uploads << (extension.start_with?(".") ? extension[1..-1] : extension) + next if extension.include?("*") + authorized_uploads << (extension.start_with?(".") ? extension[1..-1] : extension).downcase end authorized_uploads end def authorized_images - @authorized_images ||= (authorized_uploads & FileHelper.images) + authorized_uploads & FileHelper.images end def authorized_attachments - @authorized_attachments ||= (authorized_uploads - FileHelper.images) + authorized_uploads - FileHelper.images + end + + def authorizes_all_extensions? + SiteSetting.authorized_extensions.include?("*") end def authorized_extensions(upload, extension, extensions) - unless authorized = extensions.include?(extension) + return true if authorizes_all_extensions? + + unless authorized = extensions.include?(extension.downcase) message = I18n.t("upload.unauthorized", authorized_extensions: extensions.to_a.join(", ")) upload.errors.add(:original_filename, message) end + authorized end def maximum_file_size(upload, type) max_size_kb = SiteSetting.send("max_#{type}_size_kb").kilobytes + if upload.filesize > max_size_kb message = I18n.t("upload.#{type}s.too_large", max_size_kb: max_size_kb) upload.errors.add(:filesize, message) diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 997d0bda31b..a1623d167a4 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -28,7 +28,7 @@ describe UploadsController do let(:text_file) do ActionDispatch::Http::UploadedFile.new({ - filename: 'LICENSE.txt', + filename: 'LICENSE.TXT', tempfile: File.new("#{Rails.root}/LICENSE.txt") }) end @@ -39,7 +39,7 @@ describe UploadsController do context 'when authorized' do - before { SiteSetting.stubs(:authorized_extensions).returns(".png|.txt") } + before { SiteSetting.stubs(:authorized_extensions).returns(".PNG|.txt") } it 'is successful with an image' do xhr :post, :create, file: logo @@ -75,6 +75,22 @@ describe UploadsController do end + context 'when everything is authorized' do + + before { SiteSetting.stubs(:authorized_extensions).returns("*") } + + it 'is successful with an image' do + xhr :post, :create, file: logo + response.status.should eq 200 + end + + it 'is successful with an attachment' do + xhr :post, :create, file: text_file + response.status.should eq 200 + end + + end + end context 'with some files' do