FEATURE: support email attachments

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:
-        case 422: // there has been an error on the server (mostly due to FastImage)
+          bootbox.alert(data.jqXHR.responseJSON.join("\n"));
-          bootbox.alert(data.jqXHR.responseText);
           return;
       }
     }

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"
+    upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, filesize)
-    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, filesize)
+    if upload.errors.empty?
-
+      render_serialized(upload, UploadSerializer, root: false)
-    render_serialized(upload, UploadSerializer, root: false)
+    else
-
+      render status: 422, text: upload.errors.full_messages
-  rescue FastImage::ImageFetchFailure
+    end
-    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 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
+      if upload = Upload.where(id: id, url: url).first
-
+        send_file(Discourse.store.path_for(upload), filename: upload.original_filename)
-      return render nothing: true, status: 404 unless upload
+      else
-
+        render nothing: true, status: 404
-      send_file(Discourse.store.path_for(upload), filename: upload.original_filename)
+      end
-
     end
   end
 

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
+    begin
-    # TODO: Does not protect from huge uploads
+      image = build_user_image_from(file)
-    # https://github.com/discourse/discourse/pull/1512
+    rescue Discourse::InvalidParameters
-    # check the file size (note: this might also be done in the web server)
+      return render status: 422, text: I18n.t("upload.images.unknown_image_type")
-    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)
     end
 
-    raise FastImage::UnknownImageType unless SiteSetting.authorized_image?(img.file)
+    upload = Upload.create_for(user.id, image.file, image.filename, image.filesize)
-    
+
-    upload_type = params[:user_image_type]
+    if upload.errors.empty?
-    
+      case params[:user_image_type]
-    if upload_type == "avatar"
+      when "avatar"
-      upload_avatar_for(user, img)
+        upload_avatar_for(user, upload)
-    elsif upload_type == "profile_background"
+      when "profile_background"
-      upload_profile_background_for(user, img)
+        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)
+    def upload_avatar_for(user, upload)
-      upload = Upload.create_for(user.id, avatar.file, avatar.filesize)
       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)
+    def upload_profile_background_for(user, upload)
-      upload = Upload.create_for(user.id, background.file, background.filesize)
+      user.upload_profile_background(upload)
-      user.profile_background = upload.url
+      # TODO: add a resize job here
-      user.save!
+
-      
-      # TODO: maybe 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(

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, hotlinked, filename, hotlinked.size, src)
-                upload = Upload.create_for(post.user_id, file, 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, "<img src='#{url}'>")
             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

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)

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

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
+      # initialize a new upload
-      if SiteSetting.authorized_image?(file)
+      upload = Upload.new(
-        # 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!(
         user_id: user_id,
-        original_filename: file.original_filename,
+        original_filename: filename,
         filesize: filesize,
         sha1: sha1,
-        url: "",
+        url: ""
-        width: width,
-        height: height,
-        origin: origin,
       )
+      # 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

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)

app/services/uri_adapter.rb

@@ -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

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:

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}

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

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
+      # Blackberry is annoying in that it only provides HTML. We can easily extract it though
-      # 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
+    def is_in_email?
-      # Try to post the body as a reply
+      @allow_strangers = false
-      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})
 
-      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 = TopicCreator.new(
-      topic_creator = TopicCreator.new(@user,
+        @user,
-                                       Guardian.new(@user), 
+        Guardian.new(@user),
-                                       category: @category_id,
+        category: @category_id,
-                                       title: @message.subject)
+        title: @message.subject,
+      ).create
 
-      topic = topic_creator.create
+      post = create_post_with_attachments(@user, @body, topic.id)
-      post_creator = PostCreator.new(@user,
-                                     raw: @body,
-                                     topic_id: topic.id,
-                                     cooking_options: {traditional_markdown_linebreaks: true})
 
-      post_creator.create
+      EmailLog.create(
-      EmailLog.create(email_type: "topic_via_incoming_email",
+        email_type: "topic_via_incoming_email",
-            to_address: @message.to.first,
+        to_address: @message.to.first,
-            topic_id: topic.id, user_id: @user.id)
+        topic_id: topic.id,
-      topic
+        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)
+        "<img src='#{upload.url}' width='#{upload.width}' height='#{upload.height}'>"
+      else
+        "<a class='attachment' href='#{upload.url}'>#{upload.original_filename}</a> (#{number_to_human_size(upload.filesize)})"
+      end
+    end
+
+    def create_post(user, options)
+      PostCreator.new(user, options).create
     end
 
   end

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

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]
+      unique_sha1 = Digest::SHA1.hexdigest("#{Time.now.to_s}#{upload.original_filename}")[0..15]
-      extension = File.extname(file.original_filename)
+      extension = File.extname(upload.original_filename)
       clean_name = "#{unique_sha1}#{extension}"
       # path
       "#{relative_base_url}/#{upload.id}/#{clean_name}"

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
+        # TODO: server localisation has no parity with client should be fixed
-        # 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/md5.js",
-              "vendor/assets/javascripts/lodash.js",
+      "vendor/assets/javascripts/lodash.js",
-              "vendor/assets/javascripts/Markdown.Converter.js",
+      "vendor/assets/javascripts/Markdown.Converter.js",
-              "lib/headless-ember.js",
+      "lib/headless-ember.js",
-              "vendor/assets/javascripts/rsvp.js",
+      "vendor/assets/javascripts/rsvp.js",
-              Rails.configuration.ember.handlebars_location)
+      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",
+      "vendor/assets/javascripts/better_markdown.js",
-              "app/assets/javascripts/defer/html-sanitizer-bundle.js",
+      "app/assets/javascripts/defer/html-sanitizer-bundle.js",
-              "app/assets/javascripts/discourse/dialects/dialect.js",
+      "app/assets/javascripts/discourse/dialects/dialect.js",
-              "app/assets/javascripts/discourse/lib/utilities.js",
+      "app/assets/javascripts/discourse/lib/utilities.js",
-              "app/assets/javascripts/discourse/lib/html.js",
+      "app/assets/javascripts/discourse/lib/html.js",
-              "app/assets/javascripts/discourse/lib/markdown.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
 

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

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({
+    ActionDispatch::Http::UploadedFile.new({ filename: 'logo.png', tempfile: logo })
-      filename: 'logo.png',
-      tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png")
-    })
   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)
+        avatar_file.source.should == :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"
       end
     end
 
     context "when file is in the form of a URL" do
       let(:avatar_file) { AvatarUploadService.new(url, :url) }
 
-      before :each do
+      before { FileHelper.stubs(:download).returns(logo) }
-        UriAdapter.any_instance.stubs(:open).returns StringIO.new(fixture_file("images/logo.png"))
-      end
 
       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)
+        avatar_file.source.should == :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"
       end
     end
   end

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 '<div class="lightbox-wrapper"><a href="/uploads/default/1/1234567890123456.jpg" class="lightbox" title="uploaded.jpg"><img src="/uploads/default/_optimized/da3/9a3/ee5e6b4b0d_690x1380.jpg" width="690" height="1380"><div class="meta">
+        cpp.html.should match_html '<div class="lightbox-wrapper"><a href="/uploads/default/1/1234567890123456.jpg" class="lightbox" title="logo.png"><img src="/uploads/default/_optimized/da3/9a3/ee5e6b4b0d_690x1380.png" width="690" height="1380"><div class="meta">
-<span class="filename">uploaded.jpg</span><span class="informations">1000x2000 1.21 KB</span><span class="expand"></span>
+<span class="filename">logo.png</span><span class="informations">1000x2000 1.21 KB</span><span class="expand"></span>
 </div></a></div>'
         cpp.should be_dirty
       end

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<img src='\/uploads\/default\/\d+\/\w{16}\.png' width='289' height='126'>\n/)
       end
+
     end
 
   end

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
+  let(:uploaded_file) { File.new("#{Rails.root}/spec/fixtures/images/logo.png") }
-    ActionDispatch::Http::UploadedFile.new({
-      filename: 'logo.png',
-      tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png")
-    })
-  end
 
   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

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

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
 

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({
+        ActionDispatch::Http::UploadedFile.new({ filename: 'logo.png', tempfile: logo })
-          filename: 'logo.png',
-          tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png")
-        })
       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)
+          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 413
-        end
-
-        it 'rejects unauthorized images' do
-          SiteSetting.stubs(:authorized_image?).returns(false)
           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
+        it 'rejects unauthorized images' do
-          xhr :post, :upload_user_image, username: user.username, file: user_image, user_image_type: "asdf"
+          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
+        before { UsersController.any_instance.stubs(:is_api?).returns(true) }
-          UsersController.any_instance.stubs(:is_api?).returns(true)
-        end
 
         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
+          before { FileHelper.stubs(:download).returns(logo) }
-            SiteSetting.stubs(:authorized_image?).returns(false)
+
+          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
+          it 'rejects unauthorized images' do
-            xhr :post, :upload_user_image, username: user.username, file: user_image_url, user_image_type: "asdf"
+            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

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

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

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

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
+  let(:image_path) { "#{Rails.root}/spec/fixtures/images/logo.png" }
-    ActionDispatch::Http::UploadedFile.new({
+  let(:image) { File.new(image_path) }
-      filename: 'logo.png',
+  let(:image_filename) { File.basename(image_path) }
-      tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png")
+  let(:image_filesize) { File.size(image_path) }
-    })
+  let(:image_sha1) { Digest::SHA1.file(image).hexdigest }
-  end
 
-  let(:image_sha1) { Digest::SHA1.file(image.tempfile).hexdigest }
+  let(:attachment_path) { __FILE__ }
-  let(:image_filesize) { File.size(image.tempfile) }
+  let(:attachment) { File.new(attachment_path) }
-
+  let(:attachment_filename) { File.basename(attachment_path) }
-  let(:attachment) do
+  let(:attachment_filesize) { File.size(attachment_path) }
-    ActionDispatch::Http::UploadedFile.new({
-      filename: File.basename(__FILE__),
-      tempfile: File.new(__FILE__)
-    })
-  end
-
-  let(:attachment_filesize) { File.size(attachment.tempfile) }
 
   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.expects(:save).never
-      Upload.create_for(user_id, image, image_filesize).should == upload
+      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)
+      image.expects(:rewind).twice
-      Upload.create_for(user_id, image, image_filesize)
+      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)
+      FileHelper.expects(:is_image?).returns(true)
-      Upload.expects(:create!).never
+      Upload.expects(:save).never
-      expect { Upload.create_for(user_id, attachment, attachment_filesize) }.to raise_error(FastImage::UnknownImageType)
+      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.original_filename.should == image_filename
-      upload.filesize.should == File.size(image.tempfile)
+      upload.filesize.should == image_filesize
-      upload.sha1.should == Digest::SHA1.file(image.tempfile).hexdigest
+      upload.sha1.should == image_sha1
       upload.width.should == 244
       upload.height.should == 66
       upload.url.should == url

spec/services/uri_adapter_spec.rb

@@ -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