From 2505d18aa971c494c67a284ebb71f65080f34bfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 14 Apr 2014 22:55:57 +0200 Subject: [PATCH] FEATURE: support email attachments --- .../javascripts/discourse/lib/utilities.js | 7 +- app/controllers/uploads_controller.rb | 38 ++--- app/controllers/users_controller.rb | 83 +++++------ app/jobs/regular/pull_hotlinked_images.rb | 26 +--- app/jobs/scheduled/poll_mailbox.rb | 1 + app/models/site_setting.rb | 22 --- app/models/upload.rb | 61 +++++--- app/models/user.rb | 9 +- app/services/uri_adapter.rb | 64 -------- config/locales/server.en.yml | 1 + config/routes.rb | 2 +- lib/avatar_upload_service.rb | 34 +---- lib/email/receiver.rb | 140 +++++++++++------- lib/file_helper.rb | 34 +++++ lib/file_store/local_store.rb | 4 +- lib/pretty_text.rb | 32 ++-- lib/validators/upload_validator.rb | 80 ++++++++++ spec/components/avatar_upload_service_spec.rb | 58 ++++---- spec/components/cooked_post_processor_spec.rb | 4 +- spec/components/email/receiver_spec.rb | 7 +- .../components/file_store/local_store_spec.rb | 11 +- spec/components/file_store/s3_store_spec.rb | 4 +- spec/controllers/uploads_controller_spec.rb | 9 +- spec/controllers/users_controller_spec.rb | 71 ++++----- spec/fabricators/upload_fabricator.rb | 4 +- spec/models/optimized_image_spec.rb | 10 +- spec/models/site_setting_spec.rb | 22 --- spec/models/upload_spec.rb | 60 ++++---- spec/services/uri_adapter_spec.rb | 72 --------- 29 files changed, 432 insertions(+), 538 deletions(-) delete mode 100644 app/services/uri_adapter.rb create mode 100644 lib/file_helper.rb create mode 100644 lib/validators/upload_validator.rb delete mode 100644 spec/services/uri_adapter_spec.rb diff --git a/app/assets/javascripts/discourse/lib/utilities.js b/app/assets/javascripts/discourse/lib/utilities.js index 71dbe1952..200b3bbb9 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js +++ b/app/assets/javascripts/discourse/lib/utilities.js @@ -261,15 +261,16 @@ Discourse.Utilities = { switch (data.jqXHR.status) { // cancel from the user case 0: return; + // entity too large, usually returned from the web server case 413: var maxSizeKB = Discourse.SiteSettings.max_image_size_kb; bootbox.alert(I18n.t('post.errors.image_too_large', { max_size_kb: maxSizeKB })); return; + // the error message is provided by the server - case 415: // media type not authorized - case 422: // there has been an error on the server (mostly due to FastImage) - bootbox.alert(data.jqXHR.responseText); + case 422: + bootbox.alert(data.jqXHR.responseJSON.join("\n")); return; } } diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index e501d4a0e..02cb0c6cb 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -5,45 +5,29 @@ class UploadsController < ApplicationController def create file = params[:file] || params[:files].first - # check if the extension is allowed - unless SiteSetting.authorized_upload?(file) - text = I18n.t("upload.unauthorized", authorized_extensions: SiteSetting.authorized_extensions.gsub("|", ", ")) - return render status: 415, text: text - end - - # check the file size (note: this might also be done in the web server) filesize = File.size(file.tempfile) - type = SiteSetting.authorized_image?(file) ? "image" : "attachment" - max_size_kb = SiteSetting.send("max_#{type}_size_kb").kilobytes - return render status: 413, text: I18n.t("upload.#{type}s.too_large", max_size_kb: max_size_kb) if filesize > max_size_kb + upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, filesize) - upload = Upload.create_for(current_user.id, file, filesize) - - render_serialized(upload, UploadSerializer, root: false) - - rescue FastImage::ImageFetchFailure - render status: 422, text: I18n.t("upload.images.fetch_failure") - rescue FastImage::UnknownImageType - render status: 422, text: I18n.t("upload.images.unknown_image_type") - rescue FastImage::SizeNotFound - render status: 422, text: I18n.t("upload.images.size_not_found") + if upload.errors.empty? + render_serialized(upload, UploadSerializer, root: false) + else + render status: 422, text: upload.errors.full_messages + end end def show RailsMultisite::ConnectionManagement.with_connection(params[:site]) do |db| - return render nothing: true, status: 404 unless Discourse.store.internal? id = params[:id].to_i url = request.fullpath # the "url" parameter is here to prevent people from scanning the uploads using the id - upload = Upload.where(id: id, url: url).first - - return render nothing: true, status: 404 unless upload - - send_file(Discourse.store.path_for(upload), filename: upload.original_filename) - + if upload = Upload.where(id: id, url: url).first + send_file(Discourse.store.path_for(upload), filename: upload.original_filename) + else + render nothing: true, status: 404 + end end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 104938562..24a3d9443 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -307,14 +307,13 @@ class UsersController < ApplicationController size = 128 if size > 128 size end - + + # LEGACY: used by the API def upload_avatar params[:user_image_type] = "avatar" - upload_user_image - end - + def upload_user_image params.require(:user_image_type) user = fetch_user_from_params @@ -322,39 +321,24 @@ class UsersController < ApplicationController file = params[:file] || params[:files].first - # Only allow url uploading for API users - # TODO: Does not protect from huge uploads - # https://github.com/discourse/discourse/pull/1512 - # check the file size (note: this might also be done in the web server) - img = build_user_image_from(file) - upload_policy = AvatarUploadPolicy.new(img) - - if upload_policy.too_big? - return render status: 413, text: I18n.t("upload.images.too_large", - max_size_kb: upload_policy.max_size_kb) + begin + image = build_user_image_from(file) + rescue Discourse::InvalidParameters + return render status: 422, text: I18n.t("upload.images.unknown_image_type") end - raise FastImage::UnknownImageType unless SiteSetting.authorized_image?(img.file) - - upload_type = params[:user_image_type] - - if upload_type == "avatar" - upload_avatar_for(user, img) - elsif upload_type == "profile_background" - upload_profile_background_for(user, img) + upload = Upload.create_for(user.id, image.file, image.filename, image.filesize) + + if upload.errors.empty? + case params[:user_image_type] + when "avatar" + upload_avatar_for(user, upload) + when "profile_background" + upload_profile_background_for(user, upload) + end else - render status: 422, text: "" + render status: 422, text: upload.errors.full_messages end - - - rescue Discourse::InvalidParameters - render status: 422, text: I18n.t("upload.images.unknown_image_type") - rescue FastImage::ImageFetchFailure - render status: 422, text: I18n.t("upload.images.fetch_failure") - rescue FastImage::UnknownImageType - render status: 422, text: I18n.t("upload.images.unknown_image_type") - rescue FastImage::SizeNotFound - render status: 422, text: I18n.t("upload.images.size_not_found") end def toggle_avatar @@ -367,21 +351,23 @@ class UsersController < ApplicationController render nothing: true end - + def clear_profile_background user = fetch_user_from_params guardian.ensure_can_edit!(user) - + user.profile_background = "" user.save! - + render nothing: true end - + def destroy @user = fetch_user_from_params guardian.ensure_can_delete_user!(@user) + UserDestroyer.new(current_user).destroy(@user, {delete_posts: true, context: params[:context]}) + render json: success_json end @@ -403,31 +389,28 @@ class UsersController < ApplicationController def build_user_image_from(file) source = if file.is_a?(String) - is_api? ? :url : (raise FastImage::UnknownImageType) + is_api? ? :url : (raise Discourse::InvalidParameters) else :image end + AvatarUploadService.new(file, source) end - def upload_avatar_for(user, avatar) - upload = Upload.create_for(user.id, avatar.file, avatar.filesize) + def upload_avatar_for(user, upload) user.upload_avatar(upload) - Jobs.enqueue(:generate_avatars, user_id: user.id, upload_id: upload.id) + render json: { url: upload.url, width: upload.width, height: upload.height } end - - def upload_profile_background_for(user, background) - upload = Upload.create_for(user.id, background.file, background.filesize) - user.profile_background = upload.url - user.save! - - # TODO: maybe add a resize job here - + + def upload_profile_background_for(user, upload) + user.upload_profile_background(upload) + # TODO: add a resize job here + render json: { url: upload.url, width: upload.width, height: upload.height } end - + def respond_to_suspicious_request if suspicious?(params) render( diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 47f22ea5b..078dcfa48 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -1,4 +1,5 @@ require_dependency 'url_helper' +require_dependency 'file_helper' module Jobs @@ -30,14 +31,13 @@ module Jobs begin # have we already downloaded that file? if !downloaded_urls.include?(src) - hotlinked = download(src) + hotlinked = FileHelper.download(src, @max_size, "discourse-hotlinked") rescue Discourse::InvalidParameters if hotlinked.try(:size) <= @max_size filename = File.basename(URI.parse(src).path) - file = ActionDispatch::Http::UploadedFile.new(tempfile: hotlinked, filename: filename) - upload = Upload.create_for(post.user_id, file, hotlinked.size, src) + upload = Upload.create_for(post.user_id, hotlinked, filename, hotlinked.size, src) downloaded_urls[src] = upload.url else - puts "Failed to pull hotlinked image: #{src} - Image is bigger than #{@max_size}" + Rails.logger.error("Failed to pull hotlinked image: #{src} - Image is bigger than #{@max_size}") end end # have we successfully downloaded that file? @@ -59,7 +59,7 @@ module Jobs raw.gsub!(src, "") end rescue => e - puts "Failed to pull hotlinked image: #{src}\n" + e.message + "\n" + e.backtrace.join("\n") + Rails.logger.error("Failed to pull hotlinked image: #{src}\n" + e.message + "\n" + e.backtrace.join("\n")) ensure # close & delete the temp file hotlinked && hotlinked.close! @@ -87,22 +87,6 @@ module Jobs !src.start_with?(Discourse.asset_host || Discourse.base_url_no_prefix) end - def download(url) - return if @max_size <= 0 - extension = File.extname(URI.parse(url).path) - tmp = Tempfile.new(["discourse-hotlinked", extension]) - - File.open(tmp.path, "wb") do |f| - hotlinked = open(url, "rb", read_timeout: 5) - while f.size <= @max_size && data = hotlinked.read(@max_size) - f.write(data) - end - hotlinked.close! - end - - tmp - end - end end diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index b17414de2..eacd09588 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -49,6 +49,7 @@ module Jobs handle_mail(mail) end end + pop.finish end rescue Net::POPAuthenticationError => e # inform admins about the error (1 message per hour to prevent too much SPAM) diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index a7945e170..2ed664a5e 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -72,28 +72,6 @@ class SiteSetting < ActiveRecord::Base .first end - def self.authorized_uploads - authorized_extensions.tr(" ", "") - .split("|") - .map { |extension| (extension.start_with?(".") ? extension[1..-1] : extension).gsub(".", "\.") } - end - - def self.authorized_upload?(file) - authorized_uploads.count > 0 && file.original_filename =~ /\.(#{authorized_uploads.join("|")})$/i - end - - def self.images - @images ||= Set.new ["jpg", "jpeg", "png", "gif", "tif", "tiff", "bmp"] - end - - def self.authorized_images - authorized_uploads.select { |extension| images.include?(extension) } - end - - def self.authorized_image?(file) - authorized_images.count > 0 && file.original_filename =~ /\.(#{authorized_images.join("|")})$/i - end - def self.scheme use_https? ? "https" : "http" end diff --git a/app/models/upload.rb b/app/models/upload.rb index d4733e63a..a2c5917d8 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -1,5 +1,7 @@ require "digest/sha1" -require "image_sizer" +require_dependency "image_sizer" +require_dependency "file_helper" +require_dependency "validators/upload_validator" class Upload < ActiveRecord::Base belongs_to :user @@ -12,6 +14,8 @@ class Upload < ActiveRecord::Base validates_presence_of :filesize validates_presence_of :original_filename + validates_with ::Validators::UploadValidator + def thumbnail(width = self.width, height = self.height) optimized_images.where(width: width, height: height).first end @@ -42,9 +46,9 @@ class Upload < ActiveRecord::Base File.extname(original_filename) end - def self.create_for(user_id, file, filesize, origin = nil) + def self.create_for(user_id, file, filename, filesize, origin = nil) # compute the sha - sha1 = Digest::SHA1.file(file.tempfile).hexdigest + sha1 = Digest::SHA1.file(file).hexdigest # check if the file has already been uploaded upload = Upload.where(sha1: sha1).first # delete the previously uploaded file if there's been an error @@ -54,37 +58,50 @@ class Upload < ActiveRecord::Base end # create the upload unless upload - # deal with width & height for images - if SiteSetting.authorized_image?(file) - # retrieve image info - image_info = FastImage.new(file.tempfile, raise_on_failure: true) - # compute image aspect ratio - width, height = ImageSizer.resize(*image_info.size) - # make sure we're at the beginning of the file (FastImage is moving the pointer) - file.rewind - end - # trim the origin if any - origin = origin[0...1000] if origin - # create a db record (so we can use the id) - upload = Upload.create!( + # initialize a new upload + upload = Upload.new( user_id: user_id, - original_filename: file.original_filename, + original_filename: filename, filesize: filesize, sha1: sha1, - url: "", - width: width, - height: height, - origin: origin, + url: "" ) + # trim the origin if any + upload.origin = origin[0...1000] if origin + + # deal with width & height for images + if FileHelper.is_image?(filename) + begin + # retrieve image info + image_info = FastImage.new(file, raise_on_failure: true) + # compute image aspect ratio + upload.width, upload.height = ImageSizer.resize(*image_info.size) + # make sure we're at the beginning of the file (FastImage moves the pointer) + file.rewind + rescue FastImage::ImageFetchFailure + upload.errors.add(:base, I18n.t("upload.images.fetch_failure")) + rescue FastImage::UnknownImageType + upload.errors.add(:base, I18n.t("upload.images.unknown_image_type")) + rescue FastImage::SizeNotFound + upload.errors.add(:base, I18n.t("upload.images.size_not_found")) + end + + return upload unless upload.errors.empty? + end + + # create a db record (so we can use the id) + return upload unless upload.save + # store the file and update its url url = Discourse.store.store_upload(file, upload) if url.present? upload.url = url upload.save else - Rails.logger.error("Failed to store upload ##{upload.id} for user ##{user_id}") + upload.errors.add(:url, I18n.t("upload.store_failure", { upload_id: upload.id, user_id: user_id })) end end + # return the uploaded file upload end diff --git a/app/models/user.rb b/app/models/user.rb index 834ef3339..bf74bb263 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -527,13 +527,18 @@ class User < ActiveRecord::Base created_at > 1.day.ago end - def upload_avatar(avatar) + def upload_avatar(upload) self.uploaded_avatar_template = nil - self.uploaded_avatar = avatar + self.uploaded_avatar = upload self.use_uploaded_avatar = true self.save! end + def upload_profile_background(upload) + self.profile_background = upload.url + self.save! + end + def generate_api_key(created_by) if api_key.present? api_key.regenerate!(created_by) diff --git a/app/services/uri_adapter.rb b/app/services/uri_adapter.rb deleted file mode 100644 index 09cfc1a18..000000000 --- a/app/services/uri_adapter.rb +++ /dev/null @@ -1,64 +0,0 @@ -# For converting urls to files -class UriAdapter - - attr_reader :target, :content, :tempfile, :original_filename - - def initialize(target) - raise Discourse::InvalidParameters unless target =~ /^https?:\/\// - - @target = Addressable::URI.parse(target) - @original_filename = ::File.basename(@target.path) - @content = download_content - @tempfile = TempfileFactory.new.generate(@original_filename) - end - - def download_content - open(target.normalize) - end - - def copy_to_tempfile(src) - while data = src.read(16.kilobytes) - tempfile.write(data) - end - src.close - tempfile.rewind - tempfile - end - - def file_size - content.size - end - - def build_uploaded_file - return if SiteSetting.max_image_size_kb.kilobytes < file_size - - copy_to_tempfile(content) - content_type = content.content_type if content.respond_to?(:content_type) - content_type ||= "text/html" - - ActionDispatch::Http::UploadedFile.new( tempfile: tempfile, - filename: original_filename, - type: content_type - ) - end -end - -# From https://github.com/thoughtbot/paperclip/blob/master/lib/paperclip/tempfile_factory.rb -class TempfileFactory - ILLEGAL_FILENAME_CHARACTERS = /^~/ - - def generate(name) - @name = name - file = Tempfile.new([basename, extension]) - file.binmode - file - end - - def extension - File.extname(@name) - end - - def basename - File.basename(@name, extension).gsub(ILLEGAL_FILENAME_CHARACTERS, '_') - end -end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 9856f1e46..ac7c7c886 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1444,6 +1444,7 @@ en: edit_reason: "We have downloaded copies of the remote images" unauthorized: "Sorry, the file you are trying to upload is not authorized (authorized extensions: %{authorized_extensions})." pasted_image_filename: "Pasted image" + store_failure: "Failed to store upload #%{upload_id} for user #%{user_id}." attachments: too_large: "Sorry, the file you are trying to upload is too big (maximum size is %{max_size_kb}%kb)." images: diff --git a/config/routes.rb b/config/routes.rb index 9a1df2aa7..89fd08378 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -187,7 +187,7 @@ Discourse::Application.routes.draw do get "users/:username/preferences/username" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/preferences/username" => "users#username", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/avatar(/:size)" => "users#avatar", constraints: {username: USERNAME_ROUTE_FORMAT} # LEGACY ROUTE - post "users/:username/preferences/avatar" => "users#upload_avatar", constraints: {username: USERNAME_ROUTE_FORMAT} + post "users/:username/preferences/avatar" => "users#upload_avatar", constraints: {username: USERNAME_ROUTE_FORMAT} # LEGACY ROUTE post "users/:username/preferences/user_image" => "users#upload_user_image", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/preferences/avatar/toggle" => "users#toggle_avatar", constraints: {username: USERNAME_ROUTE_FORMAT} put "users/:username/preferences/profile_background/clear" => "users#clear_profile_background", constraints: {username: USERNAME_ROUTE_FORMAT} diff --git a/lib/avatar_upload_service.rb b/lib/avatar_upload_service.rb index 4a19d49da..860b7a7d5 100644 --- a/lib/avatar_upload_service.rb +++ b/lib/avatar_upload_service.rb @@ -1,43 +1,23 @@ +require_dependency "file_helper" + class AvatarUploadService attr_accessor :source - attr_reader :filesize, :file + attr_reader :filesize, :filename, :file def initialize(file, source) @source = source - @file , @filesize = construct(file) + @file, @filename, @filesize = construct(file) end def construct(file) case source when :url - build_from_url(file) + tmp = FileHelper.download(file, SiteSetting.max_image_size_kb.kilobytes, "discourse-avatar") + [tmp, File.basename(URI.parse(file).path), File.size(tmp)] when :image - [file, File.size(file.tempfile)] + [file.tempfile, file.original_filename, File.size(file.tempfile)] end end - private - - def build_from_url(url) - temp = ::UriAdapter.new(url) - return temp.build_uploaded_file, temp.file_size - end - -end - -class AvatarUploadPolicy - - def initialize(avatar) - @avatar = avatar - end - - def max_size_kb - SiteSetting.max_image_size_kb.kilobytes - end - - def too_big? - @avatar.filesize > max_size_kb - end - end diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 30eecb076..ca28308fd 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -3,8 +3,11 @@ # module Email + class Receiver + include ActionView::Helpers::NumberHelper + class ProcessingError < StandardError; end class EmailUnparsableError < ProcessingError; end class EmptyEmailError < ProcessingError; end @@ -18,28 +21,11 @@ module Email @raw = raw end - def is_in_email? - @allow_strangers = false - if SiteSetting.email_in and SiteSetting.email_in_address == @message.to.first - @category_id = SiteSetting.email_in_category.to_i - return true - end - - category = Category.find_by_email(@message.to.first) - return false if not category - - @category_id = category.id - @allow_strangers = category.email_in_allow_strangers - return true - - end - def process raise EmptyEmailError if @raw.blank? @message = Mail.new(@raw) - # First remove the known discourse stuff. parse_body raise EmptyEmailError if @body.blank? @@ -48,18 +34,17 @@ module Email @body = EmailReplyParser.read(@body).visible_text.force_encoding('UTF-8') discourse_email_parser - raise EmailUnparsableError if @body.blank? if is_in_email? @user = User.find_by_email(@message.from.first) - if @user.blank? and @allow_strangers + if @user.blank? && @allow_strangers wrap_body_in_quote @user = Discourse.system_user end raise UserNotFoundError if @user.blank? - raise UserNotSufficientTrustLevelError.new @user if not @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) + raise UserNotSufficientTrustLevelError.new @user unless @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) create_new_topic else @@ -81,12 +66,6 @@ module Email private - def wrap_body_in_quote - @body = "[quote=\"#{@message.from.first}\"] -#{@body} -[/quote]" - end - def parse_body html = nil @@ -102,7 +81,7 @@ module Email if @message.content_type =~ /text\/html/ if defined? @message.charset - html = @message.body.decoded.force_encoding(@message.charset).encode("UTF-8").to_s + html = @message.body.decoded.force_encoding(@message.charset).encode("UTF-8").to_s else html = @message.body.to_s end @@ -127,12 +106,11 @@ module Email # If we have an HTML message, strip the markup doc = Nokogiri::HTML(html) - # Blackberry is annoying in that it only provides HTML. We can easily - # extract it though + # Blackberry is annoying in that it only provides HTML. We can easily extract it though content = doc.at("#BB10_response_div") return content.text if content.present? - return doc.xpath("//text()").text + doc.xpath("//text()").text end def discourse_email_parser @@ -154,35 +132,91 @@ module Email @body.strip! end - def create_reply - # Try to post the body as a reply - creator = PostCreator.new(email_log.user, - raw: @body, - topic_id: @email_log.topic_id, - reply_to_post_number: @email_log.post.post_number, - cooking_options: {traditional_markdown_linebreaks: true}) + def is_in_email? + @allow_strangers = false - creator.create + if SiteSetting.email_in && SiteSetting.email_in_address == @message.to.first + @category_id = SiteSetting.email_in_category.to_i + return true + end + + category = Category.find_by_email(@message.to.first) + return false unless category + + @category_id = category.id + @allow_strangers = category.email_in_allow_strangers + + true + end + + def wrap_body_in_quote + @body = "[quote=\"#{@message.from.first}\"] +#{@body} +[/quote]" + end + + def create_reply + create_post_with_attachments(email_log.user, @body, @email_log.topic_id, @email_log.post.post_number) end def create_new_topic - # Try to post the body as a reply - topic_creator = TopicCreator.new(@user, - Guardian.new(@user), - category: @category_id, - title: @message.subject) + topic = TopicCreator.new( + @user, + Guardian.new(@user), + category: @category_id, + title: @message.subject, + ).create - topic = topic_creator.create - post_creator = PostCreator.new(@user, - raw: @body, - topic_id: topic.id, - cooking_options: {traditional_markdown_linebreaks: true}) + post = create_post_with_attachments(@user, @body, topic.id) - post_creator.create - EmailLog.create(email_type: "topic_via_incoming_email", - to_address: @message.to.first, - topic_id: topic.id, user_id: @user.id) - topic + EmailLog.create( + email_type: "topic_via_incoming_email", + to_address: @message.to.first, + topic_id: topic.id, + user_id: @user.id, + ) + + post + end + + def create_post_with_attachments(user, raw, topic_id, reply_to_post_number=nil) + options = { + raw: raw, + topic_id: topic_id, + cooking_options: { traditional_markdown_linebreaks: true }, + } + options[:reply_to_post_number] = reply_to_post_number if reply_to_post_number + + # deal with attachments + @message.attachments.each do |attachment| + tmp = Tempfile.new("discourse-email-attachment") + begin + # read attachment + File.open(tmp.path, "w+b") { |f| f.write attachment.body.decoded } + # create the upload for the user + upload = Upload.create_for(user.id, tmp, attachment.filename, File.size(tmp)) + if upload && upload.errors.empty? + # TODO: should use the same code as the client to insert attachments + raw << "\n#{attachment_markdown(upload)}\n" + end + ensure + tmp.close! + end + + end + + create_post(user, options) + end + + def attachment_markdown(upload)if FileHelper.is_image?(upload.original_filename) + "" + else + "#{upload.original_filename} (#{number_to_human_size(upload.filesize)})" + end + end + + def create_post(user, options) + PostCreator.new(user, options).create end end diff --git a/lib/file_helper.rb b/lib/file_helper.rb new file mode 100644 index 000000000..77fc67500 --- /dev/null +++ b/lib/file_helper.rb @@ -0,0 +1,34 @@ +class FileHelper + + def self.is_image?(filename) + filename =~ images_regexp + end + + def self.download(url, max_file_size, tmp_file_name) + raise Discourse::InvalidParameters unless url =~ /^https?:\/\// + + extension = File.extname(URI.parse(url).path) + tmp = Tempfile.new([tmp_file_name, extension]) + + File.open(tmp.path, "wb") do |f| + avatar = open(url, "rb", read_timeout: 5) + while f.size <= max_file_size && data = avatar.read(max_file_size) + f.write(data) + end + avatar.close! + end + + tmp + end + + private + + def self.images + @@images ||= Set.new ["jpg", "jpeg", "png", "gif", "tif", "tiff", "bmp"] + end + + def self.images_regexp + @@images_regexp ||= /\.(#{images.to_a.join("|").gsub(".", "\.")})$/i + end + +end diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb index da43b48b0..78fd8755c 100644 --- a/lib/file_store/local_store.rb +++ b/lib/file_store/local_store.rb @@ -62,8 +62,8 @@ module FileStore private def get_path_for_upload(file, upload) - unique_sha1 = Digest::SHA1.hexdigest("#{Time.now.to_s}#{file.original_filename}")[0..15] - extension = File.extname(file.original_filename) + unique_sha1 = Digest::SHA1.hexdigest("#{Time.now.to_s}#{upload.original_filename}")[0..15] + extension = File.extname(upload.original_filename) clean_name = "#{unique_sha1}#{extension}" # path "#{relative_base_url}/#{upload.id}/#{clean_name}" diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 832f7def6..75686b9f1 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -10,8 +10,7 @@ module PrettyText def t(key, opts) str = I18n.t("js." + key) if opts - # TODO: server localisation has no parity with client - # should be fixed + # TODO: server localisation has no parity with client should be fixed str = str.dup opts.each do |k,v| str.gsub!("{{#{k}}}", v) @@ -31,7 +30,7 @@ module PrettyText def is_username_valid(username) return false unless username username = username.downcase - return User.exec_sql('select 1 from users where username_lower = ?', username).values.length == 1 + return User.exec_sql('SELECT 1 FROM users WHERE username_lower = ?', username).values.length == 1 end end @@ -53,12 +52,13 @@ module PrettyText ctx["helpers"] = Helpers.new ctx_load(ctx, - "vendor/assets/javascripts/md5.js", - "vendor/assets/javascripts/lodash.js", - "vendor/assets/javascripts/Markdown.Converter.js", - "lib/headless-ember.js", - "vendor/assets/javascripts/rsvp.js", - Rails.configuration.ember.handlebars_location) + "vendor/assets/javascripts/md5.js", + "vendor/assets/javascripts/lodash.js", + "vendor/assets/javascripts/Markdown.Converter.js", + "lib/headless-ember.js", + "vendor/assets/javascripts/rsvp.js", + Rails.configuration.ember.handlebars_location + ) ctx.eval("var Discourse = {}; Discourse.SiteSettings = {};") ctx.eval("var window = {}; window.devicePixelRatio = 2;") # hack to make code think stuff is retina @@ -67,12 +67,13 @@ module PrettyText decorate_context(ctx) ctx_load(ctx, - "vendor/assets/javascripts/better_markdown.js", - "app/assets/javascripts/defer/html-sanitizer-bundle.js", - "app/assets/javascripts/discourse/dialects/dialect.js", - "app/assets/javascripts/discourse/lib/utilities.js", - "app/assets/javascripts/discourse/lib/html.js", - "app/assets/javascripts/discourse/lib/markdown.js") + "vendor/assets/javascripts/better_markdown.js", + "app/assets/javascripts/defer/html-sanitizer-bundle.js", + "app/assets/javascripts/discourse/dialects/dialect.js", + "app/assets/javascripts/discourse/lib/utilities.js", + "app/assets/javascripts/discourse/lib/html.js", + "app/assets/javascripts/discourse/lib/markdown.js" + ) Dir["#{Rails.root}/app/assets/javascripts/discourse/dialects/**.js"].each do |dialect| unless dialect =~ /\/dialect\.js$/ @@ -111,6 +112,7 @@ module PrettyText return @ctx if @ctx @ctx = create_new_context end + @ctx end diff --git a/lib/validators/upload_validator.rb b/lib/validators/upload_validator.rb new file mode 100644 index 000000000..4ba0710d1 --- /dev/null +++ b/lib/validators/upload_validator.rb @@ -0,0 +1,80 @@ +require_dependency "file_helper" + +module Validators; end + +class Validators::UploadValidator < ActiveModel::Validator + + def validate(upload) + extension = File.extname(upload.original_filename)[1..-1] + + if is_authorized?(upload, extension) + if FileHelper.is_image?(upload.original_filename) + authorized_image_extension(upload, extension) + maximum_image_file_size(upload) + else + authorized_attachment_extension(upload, extension) + maximum_attachment_file_size(upload) + end + end + end + + def is_authorized?(upload, extension) + authorized_extensions(upload, extension, authorized_uploads) + end + + def authorized_image_extension(upload, extension) + authorized_extensions(upload, extension, authorized_images) + end + + def maximum_image_file_size(upload) + maximum_file_size(upload, "image") + end + + def authorized_attachment_extension(upload, extension) + authorized_extensions(upload, extension, authorized_attachments) + end + + def maximum_attachment_file_size(upload) + maximum_file_size(upload, "attachment") + end + + private + + def authorized_uploads + authorized_uploads = Set.new + + SiteSetting.authorized_extensions + .tr(" ", "") + .split("|") + .each do |extension| + authorized_uploads << (extension.start_with?(".") ? extension[1..-1] : extension) + end + + authorized_uploads + end + + def authorized_images + @authorized_images ||= (authorized_uploads & FileHelper.images) + end + + def authorized_attachments + @authorized_attachments ||= (authorized_uploads - FileHelper.images) + end + + def authorized_extensions(upload, extension, extensions) + unless authorized = extensions.include?(extension) + 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) + end + end + +end diff --git a/spec/components/avatar_upload_service_spec.rb b/spec/components/avatar_upload_service_spec.rb index 6248470bb..c515bbb34 100644 --- a/spec/components/avatar_upload_service_spec.rb +++ b/spec/components/avatar_upload_service_spec.rb @@ -2,11 +2,11 @@ require "spec_helper" require "avatar_upload_service" describe AvatarUploadService do + + let(:logo) { File.new("#{Rails.root}/spec/fixtures/images/logo.png") } + let(:file) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) + ActionDispatch::Http::UploadedFile.new({ filename: 'logo.png', tempfile: logo }) end let(:url) { "http://cdn.discourse.org/assets/logo.png" } @@ -16,49 +16,41 @@ describe AvatarUploadService do let(:avatar_file) { AvatarUploadService.new(file, :image) } it "should have a filesize" do - expect(avatar_file.filesize).to eq(2290) + avatar_file.filesize.should == 2290 + end + + it "should have a filename" do + avatar_file.filename.should == "logo.png" + end + + it "should have a file" do + avatar_file.file.should == file.tempfile end it "should have a source as 'image'" do - expect(avatar_file.source).to eq(:image) - end - - it "is an instance of File class" do - file = avatar_file.file - expect(file.tempfile).to be_instance_of File - end - - it "returns the file object built from File" do - file = avatar_file.file - file.should be_instance_of(ActionDispatch::Http::UploadedFile) - file.original_filename.should == "logo.png" + avatar_file.source.should == :image end end context "when file is in the form of a URL" do let(:avatar_file) { AvatarUploadService.new(url, :url) } - before :each do - UriAdapter.any_instance.stubs(:open).returns StringIO.new(fixture_file("images/logo.png")) - end + before { FileHelper.stubs(:download).returns(logo) } it "should have a filesize" do - expect(avatar_file.filesize).to eq(2290) + avatar_file.filesize.should == 2290 + end + + it "should have a filename" do + avatar_file.filename.should == "logo.png" + end + + it "should have a file" do + avatar_file.file.should == logo end it "should have a source as 'url'" do - expect(avatar_file.source).to eq(:url) - end - - it "is an instance of Tempfile class" do - file = avatar_file.file - expect(file.tempfile).to be_instance_of Tempfile - end - - it "returns the file object built from URL" do - file = avatar_file.file - file.should be_instance_of(ActionDispatch::Http::UploadedFile) - file.original_filename.should == "logo.png" + avatar_file.source.should == :url end end end diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index ec674739b..93c13a213 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -94,8 +94,8 @@ describe CookedPostProcessor do it "generates overlay information" do cpp.post_process_images - cpp.html.should match_html '
-uploaded.jpg1000x2000 1.21 KB + cpp.html.should match_html '
+logo.png1000x2000 1.21 KB
' cpp.should be_dirty end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 2d8259bde..1a0ce9e0c 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -178,15 +178,18 @@ greatest show ever created. Everyone should watch it. end describe "email with attachments" do + it "can find the message and create a post" do + user.id = -1 User.stubs(:find_by_email).returns(user) EmailLog.stubs(:for).returns(email_log) attachment_email = File.read("#{Rails.root}/spec/fixtures/emails/attachment.eml") r = Email::Receiver.new(attachment_email) - r.expects(:create_reply) + r.expects(:create_post) expect { r.process }.to_not raise_error - expect(r.body).to eq("here is an image attachment") + expect(r.body).to match(/here is an image attachment\n\n/) end + end end diff --git a/spec/components/file_store/local_store_spec.rb b/spec/components/file_store/local_store_spec.rb index b8330506b..63a6b8ac4 100644 --- a/spec/components/file_store/local_store_spec.rb +++ b/spec/components/file_store/local_store_spec.rb @@ -6,12 +6,7 @@ describe FileStore::LocalStore do let(:store) { FileStore::LocalStore.new } let(:upload) { build(:upload) } - let(:uploaded_file) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) - end + let(:uploaded_file) { File.new("#{Rails.root}/spec/fixtures/images/logo.png") } let(:optimized_image) { build(:optimized_image) } let(:avatar) { build(:upload) } @@ -40,7 +35,7 @@ describe FileStore::LocalStore do it "returns a relative url" do store.expects(:copy_file) - store.store_avatar({}, upload, 100).should == "/uploads/default/avatars/e9d/71f/5ee7c92d6d/100.jpg" + store.store_avatar({}, upload, 100).should == "/uploads/default/avatars/e9d/71f/5ee7c92d6d/100.png" end end @@ -125,7 +120,7 @@ describe FileStore::LocalStore do describe ".avatar_template" do it "is present" do - store.avatar_template(avatar).should == "/uploads/default/avatars/e9d/71f/5ee7c92d6d/{size}.jpg" + store.avatar_template(avatar).should == "/uploads/default/avatars/e9d/71f/5ee7c92d6d/{size}.png" end end diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb index 40ebc71c1..d47c7f69c 100644 --- a/spec/components/file_store/s3_store_spec.rb +++ b/spec/components/file_store/s3_store_spec.rb @@ -64,7 +64,7 @@ describe FileStore::S3Store do it "returns an absolute schemaless url" do avatar.stubs(:id).returns(42) - store.store_avatar(avatar_file, avatar, 100).should == "//s3_upload_bucket.s3.amazonaws.com/avatars/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98/100.jpg" + store.store_avatar(avatar_file, avatar, 100).should == "//s3_upload_bucket.s3.amazonaws.com/avatars/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98/100.png" end end @@ -116,7 +116,7 @@ describe FileStore::S3Store do describe ".avatar_template" do it "is present" do - store.avatar_template(avatar).should == "//s3_upload_bucket.s3.amazonaws.com/avatars/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98/{size}.jpg" + store.avatar_template(avatar).should == "//s3_upload_bucket.s3.amazonaws.com/avatars/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98/{size}.png" end end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index d45961804..997d0bda3 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -57,7 +57,7 @@ describe UploadsController do it 'rejects the upload' do xhr :post, :create, file: text_file - response.status.should eq 413 + response.status.should eq 422 end end @@ -70,7 +70,7 @@ describe UploadsController do it 'rejects the upload' do xhr :post, :create, file: text_file - response.status.should eq 415 + response.status.should eq 422 end end @@ -112,9 +112,12 @@ describe UploadsController do end it 'uses send_file' do - Fabricate(:attachment) + upload = build(:upload) + Upload.expects(:where).with(id: 42, url: "/uploads/default/42/66b3ed1503efc936.zip").returns([upload]) + controller.stubs(:render) controller.expects(:send_file) + get :show, site: "default", id: 42, sha: "66b3ed1503efc936", extension: "zip" end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 0b401e980..689b1f59d 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1099,25 +1099,23 @@ describe UsersController do it 'raises an error when not logged in' do lambda { xhr :put, :upload_user_image, username: 'asdf' }.should raise_error(Discourse::NotLoggedIn) end - context 'while logged in' do let!(:user) { log_in } + let(:logo) { File.new("#{Rails.root}/spec/fixtures/images/logo.png") } + let(:user_image) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) + ActionDispatch::Http::UploadedFile.new({ filename: 'logo.png', tempfile: logo }) end - + it 'raises an error without a user_image_type param' do lambda { xhr :put, :upload_user_image, username: user.username }.should raise_error(ActionController::ParameterMissing) end describe "with uploaded file" do - + it 'raises an error when you don\'t have permission to upload an user image' do Guardian.any_instance.expects(:can_edit?).with(user).returns(false) xhr :post, :upload_user_image, username: user.username, user_image_type: "avatar" @@ -1125,19 +1123,14 @@ describe UsersController do end it 'rejects large images' do - AvatarUploadPolicy.any_instance.stubs(:too_big?).returns(true) - xhr :post, :upload_user_image, username: user.username, file: user_image, user_image_type: "avatar" - response.status.should eq 413 - end - - it 'rejects unauthorized images' do - SiteSetting.stubs(:authorized_image?).returns(false) + SiteSetting.stubs(:max_image_size_kb).returns(1) xhr :post, :upload_user_image, username: user.username, file: user_image, user_image_type: "avatar" response.status.should eq 422 end - - it 'rejects requests with unknown user_image_type' do - xhr :post, :upload_user_image, username: user.username, file: user_image, user_image_type: "asdf" + + it 'rejects unauthorized images' do + SiteSetting.stubs(:authorized_extensions).returns(".txt") + xhr :post, :upload_user_image, username: user.username, file: user_image, user_image_type: "avatar" response.status.should eq 422 end @@ -1156,54 +1149,46 @@ describe UsersController do user.use_uploaded_avatar.should == true # returns the url, width and height of the uploaded image json = JSON.parse(response.body) - json['url'].should == "/uploads/default/1/1234567890123456.jpg" + json['url'].should == "/uploads/default/1/1234567890123456.png" json['width'].should == 100 json['height'].should == 200 end - + it 'is successful for profile backgrounds' do upload = Fabricate(:upload) Upload.expects(:create_for).returns(upload) xhr :post, :upload_user_image, username: user.username, file: user_image, user_image_type: "profile_background" user.reload - - user.profile_background.should == "/uploads/default/1/1234567890123456.jpg" - + + user.profile_background.should == "/uploads/default/1/1234567890123456.png" + # returns the url, width and height of the uploaded image json = JSON.parse(response.body) - json['url'].should == "/uploads/default/1/1234567890123456.jpg" + json['url'].should == "/uploads/default/1/1234567890123456.png" json['width'].should == 100 json['height'].should == 200 end - + end describe "with url" do let(:user_image_url) { "http://cdn.discourse.org/assets/logo.png" } - before :each do - UsersController.any_instance.stubs(:is_api?).returns(true) - end + before { UsersController.any_instance.stubs(:is_api?).returns(true) } describe "correct urls" do - before :each do - UriAdapter.any_instance.stubs(:open).returns StringIO.new(fixture_file("images/logo.png")) - end - - it 'rejects large images' do - AvatarUploadPolicy.any_instance.stubs(:too_big?).returns(true) - xhr :post, :upload_user_image, username: user.username, file: user_image_url, user_image_type: "profile_background" - response.status.should eq 413 - end - it 'rejects unauthorized images' do - SiteSetting.stubs(:authorized_image?).returns(false) + before { FileHelper.stubs(:download).returns(logo) } + + it 'rejects large images' do + SiteSetting.stubs(:max_image_size_kb).returns(1) xhr :post, :upload_user_image, username: user.username, file: user_image_url, user_image_type: "profile_background" response.status.should eq 422 end - it 'rejects requests with unknown user_image_type' do - xhr :post, :upload_user_image, username: user.username, file: user_image_url, user_image_type: "asdf" + it 'rejects unauthorized images' do + SiteSetting.stubs(:authorized_extensions).returns(".txt") + xhr :post, :upload_user_image, username: user.username, file: user_image_url, user_image_type: "profile_background" response.status.should eq 422 end @@ -1222,7 +1207,7 @@ describe UsersController do user.use_uploaded_avatar.should == true # returns the url, width and height of the uploaded image json = JSON.parse(response.body) - json['url'].should == "/uploads/default/1/1234567890123456.jpg" + json['url'].should == "/uploads/default/1/1234567890123456.png" json['width'].should == 100 json['height'].should == 200 end @@ -1232,11 +1217,11 @@ describe UsersController do Upload.expects(:create_for).returns(upload) xhr :post, :upload_user_image, username: user.username, file: user_image_url, user_image_type: "profile_background" user.reload - user.profile_background.should == "/uploads/default/1/1234567890123456.jpg" + user.profile_background.should == "/uploads/default/1/1234567890123456.png" # returns the url, width and height of the uploaded image json = JSON.parse(response.body) - json['url'].should == "/uploads/default/1/1234567890123456.jpg" + json['url'].should == "/uploads/default/1/1234567890123456.png" json['width'].should == 100 json['height'].should == 200 end diff --git a/spec/fabricators/upload_fabricator.rb b/spec/fabricators/upload_fabricator.rb index c8788a972..a64d7def4 100644 --- a/spec/fabricators/upload_fabricator.rb +++ b/spec/fabricators/upload_fabricator.rb @@ -1,11 +1,11 @@ Fabricator(:upload) do user sha1 "e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98" - original_filename "uploaded.jpg" + original_filename "logo.png" filesize 1234 width 100 height 200 - url "/uploads/default/1/1234567890123456.jpg" + url "/uploads/default/1/1234567890123456.png" end Fabricator(:attachment, from: :upload) do diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb index 0e663ea5f..f5e740421 100644 --- a/spec/models/optimized_image_spec.rb +++ b/spec/models/optimized_image_spec.rb @@ -42,10 +42,10 @@ describe OptimizedImage do it "works" do oi = OptimizedImage.create_for(upload, 100, 200) oi.sha1.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709" - oi.extension.should == ".jpg" + oi.extension.should == ".png" oi.width.should == 100 oi.height.should == 200 - oi.url.should == "/internally/stored/optimized/image.jpg" + oi.url.should == "/internally/stored/optimized/image.png" end end @@ -73,17 +73,17 @@ describe OptimizedImage do it "downloads a copy of the original image" do Tempfile.any_instance.expects(:close!).twice - store.expects(:download).with(upload).returns(Tempfile.new(["discourse-external", ".jpg"])) + store.expects(:download).with(upload).returns(Tempfile.new(["discourse-external", ".png"])) OptimizedImage.create_for(upload, 100, 200) end it "works" do oi = OptimizedImage.create_for(upload, 100, 200) oi.sha1.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709" - oi.extension.should == ".jpg" + oi.extension.should == ".png" oi.width.should == 100 oi.height.should == 200 - oi.url.should == "/externally/stored/optimized/image.jpg" + oi.url.should == "/externally/stored/optimized/image.png" end end diff --git a/spec/models/site_setting_spec.rb b/spec/models/site_setting_spec.rb index c0864638a..1027a1052 100644 --- a/spec/models/site_setting_spec.rb +++ b/spec/models/site_setting_spec.rb @@ -105,28 +105,6 @@ describe SiteSetting do end end - describe "authorized extensions" do - - describe "authorized_uploads" do - - it "trims spaces and leading dots" do - SiteSetting.stubs(:authorized_extensions).returns(" png | .jpeg|txt|bmp | .tar.gz") - SiteSetting.authorized_uploads.should == ["png", "jpeg", "txt", "bmp", "tar.gz"] - end - - end - - describe "authorized_images" do - - it "filters non-image out" do - SiteSetting.stubs(:authorized_extensions).returns(" png | .jpeg|txt|bmp") - SiteSetting.authorized_images.should == ["png", "jpeg", "bmp"] - end - - end - - end - describe "scheme" do it "returns http when ssl is disabled" do diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 4006bc796..ec2aeec4d 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -2,7 +2,6 @@ require 'spec_helper' require 'digest/sha1' describe Upload do - it { should belong_to :user } it { should have_many :post_uploads } @@ -10,33 +9,22 @@ describe Upload do it { should have_many :optimized_images } - it { should validate_presence_of :original_filename } - it { should validate_presence_of :filesize } - let(:upload) { build(:upload) } let(:thumbnail) { build(:optimized_image, upload: upload) } let(:user_id) { 1 } let(:url) { "http://domain.com" } - let(:image) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) - end + let(:image_path) { "#{Rails.root}/spec/fixtures/images/logo.png" } + let(:image) { File.new(image_path) } + let(:image_filename) { File.basename(image_path) } + let(:image_filesize) { File.size(image_path) } + let(:image_sha1) { Digest::SHA1.file(image).hexdigest } - let(:image_sha1) { Digest::SHA1.file(image.tempfile).hexdigest } - let(:image_filesize) { File.size(image.tempfile) } - - let(:attachment) do - ActionDispatch::Http::UploadedFile.new({ - filename: File.basename(__FILE__), - tempfile: File.new(__FILE__) - }) - end - - let(:attachment_filesize) { File.size(attachment.tempfile) } + let(:attachment_path) { __FILE__ } + let(:attachment) { File.new(attachment_path) } + let(:attachment_filename) { File.basename(attachment_path) } + let(:attachment_filesize) { File.size(attachment_path) } context ".create_thumbnail!" do @@ -62,39 +50,41 @@ describe Upload do it "does not create another upload if it already exists" do Upload.expects(:where).with(sha1: image_sha1).returns([upload]) - Upload.expects(:create!).never - Upload.create_for(user_id, image, image_filesize).should == upload + Upload.expects(:save).never + Upload.create_for(user_id, image, image_filename, image_filesize).should == upload end it "computes width & height for images" do - SiteSetting.expects(:authorized_image?).returns(true) FastImage.any_instance.expects(:size).returns([100, 200]) ImageSizer.expects(:resize) - ActionDispatch::Http::UploadedFile.any_instance.expects(:rewind) - Upload.create_for(user_id, image, image_filesize) + image.expects(:rewind).twice + Upload.create_for(user_id, image, image_filename, image_filesize) end it "does not create an upload when there is an error with FastImage" do - SiteSetting.expects(:authorized_image?).returns(true) - Upload.expects(:create!).never - expect { Upload.create_for(user_id, attachment, attachment_filesize) }.to raise_error(FastImage::UnknownImageType) + FileHelper.expects(:is_image?).returns(true) + Upload.expects(:save).never + upload = Upload.create_for(user_id, attachment, attachment_filename, attachment_filesize) + upload.errors.size.should > 0 end it "does not compute width & height for non-image" do - SiteSetting.expects(:authorized_image?).returns(false) FastImage.any_instance.expects(:size).never - Upload.create_for(user_id, image, image_filesize) + upload = Upload.create_for(user_id, attachment, attachment_filename, attachment_filesize) + upload.errors.size.should > 0 end it "saves proper information" do store = {} Discourse.expects(:store).returns(store) store.expects(:store_upload).returns(url) - upload = Upload.create_for(user_id, image, image_filesize) + + upload = Upload.create_for(user_id, image, image_filename, image_filesize) + upload.user_id.should == user_id - upload.original_filename.should == image.original_filename - upload.filesize.should == File.size(image.tempfile) - upload.sha1.should == Digest::SHA1.file(image.tempfile).hexdigest + upload.original_filename.should == image_filename + upload.filesize.should == image_filesize + upload.sha1.should == image_sha1 upload.width.should == 244 upload.height.should == 66 upload.url.should == url diff --git a/spec/services/uri_adapter_spec.rb b/spec/services/uri_adapter_spec.rb deleted file mode 100644 index 8f5ef020f..000000000 --- a/spec/services/uri_adapter_spec.rb +++ /dev/null @@ -1,72 +0,0 @@ -require 'spec_helper' - -describe UriAdapter do - let(:target) { "http://cdn.discourse.org/assets/logo.png" } - let(:response) { StringIO.new(fixture_file("images/logo.png")) } - - before :each do - response.stubs(:content_type).returns("image/png") - UriAdapter.any_instance.stubs(:open).returns(response) - end - - subject { UriAdapter.new(target) } - - describe "#initialize" do - - it "has a target" do - subject.target.should be_instance_of(Addressable::URI) - end - - it "has content" do - subject.content.should == response - end - - it "has an original_filename" do - subject.original_filename.should == "logo.png" - end - - it "has a tempfile" do - subject.tempfile.should be_instance_of Tempfile - end - - describe "it handles ugly targets" do - let(:ugly_target) { "http://cdn.discourse.org/assets/logo with spaces.png" } - subject { UriAdapter.new(ugly_target) } - - it "handles targets" do - subject.target.should be_instance_of(Addressable::URI) - end - - it "has content" do - subject.content.should == response - end - - it "has an original_filename" do - subject.original_filename.should == "logo with spaces.png" - end - - it "has a tempfile" do - subject.tempfile.should be_instance_of Tempfile - end - end - - end - - describe "#copy_to_tempfile" do - it "does not allow files bigger then max_image_size_kb" do - SiteSetting.stubs(:max_image_size_kb).returns(1) - subject.build_uploaded_file.should == nil - end - end - - describe "#build_uploaded_file" do - it "returns an uploaded file" do - file = subject.build_uploaded_file - file.should be_instance_of(ActionDispatch::Http::UploadedFile) - file.content_type.should == "image/png" - file.original_filename.should == "logo.png" - file.tempfile.should be_instance_of Tempfile - end - end - -end \ No newline at end of file