BUGFIX: uploads to S3

app/controllers/uploads_controller.rb

@@ -6,7 +6,7 @@ class UploadsController < ApplicationController
     file = params[:file] || params[:files].first
 
     filesize = File.size(file.tempfile)
-    upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, filesize)
+    upload = Upload.create_for(current_user.id, file.tempfile, file.original_filename, filesize, file.content_type)
 
     if upload.errors.empty?
       render_serialized(upload, UploadSerializer, root: false)

app/jobs/regular/pull_hotlinked_images.rb

@@ -34,7 +34,7 @@ module Jobs
               hotlinked = FileHelper.download(src, @max_size, "discourse-hotlinked") rescue Discourse::InvalidParameters
               if hotlinked.try(:size) <= @max_size
                 filename = File.basename(URI.parse(src).path)
-                upload = Upload.create_for(post.user_id, hotlinked, filename, hotlinked.size, src)
+                upload = Upload.create_for(post.user_id, hotlinked, filename, hotlinked.size, nil, src)
                 downloaded_urls[src] = upload.url
               else
                 Rails.logger.error("Failed to pull hotlinked image: #{src} - Image is bigger than #{@max_size}")

app/models/upload.rb

@@ -46,7 +46,7 @@ class Upload < ActiveRecord::Base
     File.extname(original_filename)
   end
 
-  def self.create_for(user_id, file, filename, filesize, origin = nil)
+  def self.create_for(user_id, file, filename, filesize, content_type = nil, origin = nil)
     # compute the sha
     sha1 = Digest::SHA1.file(file).hexdigest
     # check if the file has already been uploaded
@@ -93,7 +93,7 @@ class Upload < ActiveRecord::Base
       return upload unless upload.save
 
       # store the file and update its url
-      url = Discourse.store.store_upload(file, upload)
+      url = Discourse.store.store_upload(file, upload, content_type)
       if url.present?
         upload.url = url
         upload.save

lib/file_helper.rb

@@ -11,11 +11,11 @@ class FileHelper
     tmp = Tempfile.new([tmp_file_name, extension])
 
     File.open(tmp.path, "wb") do |f|
-      avatar = open(url, "rb", read_timeout: 5)
+      downloaded = open(url, "rb", read_timeout: 5)
-      while f.size <= max_file_size && data = avatar.read(max_file_size)
+      while f.size <= max_file_size && data = downloaded.read(max_file_size)
         f.write(data)
       end
-      avatar.close!
+      downloaded.close!
     end
 
     tmp

lib/file_store/base_store.rb

@@ -2,7 +2,7 @@ module FileStore
 
   class BaseStore
 
-    def store_upload(file, upload)
+    def store_upload(file, upload, content_type = nil)
     end
 
     def store_optimized_image(file, optimized_image)

lib/file_store/local_store.rb

@@ -4,7 +4,7 @@ module FileStore
 
   class LocalStore < BaseStore
 
-    def store_upload(file, upload)
+    def store_upload(file, upload, content_type = nil)
       path = get_path_for_upload(file, upload)
       store_file(file, path)
     end

lib/file_store/s3_store.rb

@@ -1,13 +1,14 @@
 require 'file_store/base_store'
+require_dependency "file_helper"
 
 module FileStore
 
   class S3Store < BaseStore
     @fog_loaded ||= require 'fog'
 
-    def store_upload(file, upload)
+    def store_upload(file, upload, content_type = nil)
       path = get_path_for_upload(file, upload)
-      store_file(file, path, upload.original_filename, file.content_type)
+      store_file(file, path, upload.original_filename, content_type)
     end
 
     def store_optimized_image(file, optimized_image)
@@ -45,17 +46,10 @@ module FileStore
     end
 
     def download(upload)
-      @open_uri_loaded ||= require 'open-uri'
-
-      extension = File.extname(upload.original_filename)
-      temp_file = Tempfile.new(["discourse-s3", extension])
       url = SiteSetting.scheme + ":" + upload.url
+      max_file_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes
 
-      File.open(temp_file.path, "wb") do |f|
+      FileHelper.download(url, max_file_size, "discourse-s3")
-        f.write(open(url, "rb", read_timeout: 5).read)
-      end
-
-      temp_file
     end
 
     def avatar_template(avatar)

spec/components/file_store/s3_store_spec.rb

@@ -7,28 +7,13 @@ describe FileStore::S3Store do
   let(:store) { FileStore::S3Store.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(:optimized_image_file) do
+  let(:optimized_image_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(:avatar) { build(:upload) }
-  let(:avatar_file) do
+  let(:avatar_file) { File.new("#{Rails.root}/spec/fixtures/images/logo-dev.png") }
-    ActionDispatch::Http::UploadedFile.new({
-      filename: 'logo-dev.png',
-      tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo-dev.png")
-    })
-  end
 
   before(:each) do
     SiteSetting.stubs(:s3_upload_bucket).returns("S3_Upload_Bucket")