FIX: uploading an existing image as a site setting

The previous fix (f43c0a5d85) wasn't working for images that were already uploaded.
The "metadata" (eg. 'for_*' and 'secure' attributes) were not added to existing uploads.

Also used 'Upload.get_from_url' is the admin/site_setting controller to properly retrieve
an upload from its URL.

Fixed the Upload::URL_REGEX to use the \h (hexadecimal) for the SHA

Follow-up-to: f43c0a5d85

app/controllers/admin/site_settings_controller.rb

@@ -17,7 +17,7 @@ class Admin::SiteSettingsController < Admin::AdminController
     raise_access_hidden_setting(id)
 
     if SiteSetting.type_supervisor.get_type(id) == :upload
-      value = Upload.find_by(url: value) || ''
+      value = Upload.get_from_url(value) || ""
     end
 
     update_existing_users = params[:updateExistingUsers].present?

app/models/upload.rb

@@ -8,7 +8,7 @@ class Upload < ActiveRecord::Base
 
   SHA1_LENGTH = 40
   SEEDED_ID_THRESHOLD = 0
-  URL_REGEX ||= /(\/original\/\dX[\/\.\w]*\/([a-zA-Z0-9]+)[\.\w]*)/
+  URL_REGEX ||= /(\/original\/\dX[\/\.\w]*\/(\h+)[\.\w]*)/
   SECURE_MEDIA_ROUTE = "secure-media-uploads"
 
   belongs_to :user

lib/upload_creator.rb

@@ -73,7 +73,6 @@ class UploadCreator
       # between uploads instead of the sha1, and to get around various
       # access/permission issues for uploads
       if !SiteSetting.secure_media
-
         # do we already have that upload?
         @upload = Upload.find_by(sha1: sha1)
 
@@ -85,6 +84,7 @@ class UploadCreator
 
         # return the previous upload if any
         if @upload
+          add_metadata!
           UserUpload.find_or_create_by!(user_id: user_id, upload_id: @upload.id) if user_id
           return @upload
         end
@@ -121,14 +121,7 @@ class UploadCreator
         @upload.width, @upload.height = @image_info.size
       end
 
-      @upload.for_private_message = true if @opts[:for_private_message]
-      @upload.for_group_message   = true if @opts[:for_group_message]
-      @upload.for_theme           = true if @opts[:for_theme]
-      @upload.for_export          = true if @opts[:for_export]
-      @upload.for_site_setting    = true if @opts[:for_site_setting]
-      @upload.for_gravatar        = true if @opts[:for_gravatar]
-      @upload.secure = UploadSecurity.new(@upload, @opts).should_be_secure?
-
+      add_metadata!
       return @upload unless @upload.save
 
       # store the file and update its url
@@ -375,4 +368,14 @@ class UploadCreator
     @@svg_whitelist_xpath ||= "//*[#{WHITELISTED_SVG_ELEMENTS.map { |e| "name()!='#{e}'" }.join(" and ") }]"
   end
 
+  def add_metadata!
+    @upload.for_private_message = true if @opts[:for_private_message]
+    @upload.for_group_message   = true if @opts[:for_group_message]
+    @upload.for_theme           = true if @opts[:for_theme]
+    @upload.for_export          = true if @opts[:for_export]
+    @upload.for_site_setting    = true if @opts[:for_site_setting]
+    @upload.for_gravatar        = true if @opts[:for_gravatar]
+    @upload.secure = UploadSecurity.new(@upload, @opts).should_be_secure?
+  end
+
 end

spec/requests/uploads_controller_spec.rb

@@ -17,18 +17,11 @@ describe UploadsController do
       end
 
       let(:logo_file) { file_from_fixtures("logo.png") }
-      let(:logo) do
-        Rack::Test::UploadedFile.new(logo_file)
-      end
       let(:logo_filename) { File.basename(logo_file) }
 
-      let(:fake_jpg) do
-        Rack::Test::UploadedFile.new(file_from_fixtures("fake.jpg"))
-      end
-
-      let(:text_file) do
-        Rack::Test::UploadedFile.new(File.new("#{Rails.root}/LICENSE.txt"))
-      end
+      let(:logo) { Rack::Test::UploadedFile.new(logo_file) }
+      let(:fake_jpg) { Rack::Test::UploadedFile.new(file_from_fixtures("fake.jpg")) }
+      let(:text_file) { Rack::Test::UploadedFile.new(File.new("#{Rails.root}/LICENSE.txt")) }
 
       it 'expects a type' do
         post "/uploads.json", params: { file: logo }
@@ -44,9 +37,13 @@ describe UploadsController do
 
       it 'returns "raw" url for site settings' do
         set_cdn_url "https://awesome.com"
+
+        upload = UploadCreator.new(logo_file, "logo.png").create_for(-1)
+        logo = Rack::Test::UploadedFile.new(file_from_fixtures("logo.png"))
+
         post "/uploads.json", params: { file: logo, type: "site_setting", for_site_setting: "true" }
         expect(response.status).to eq 200
-        expect(response.parsed_body["url"]).to start_with("/uploads/default/")
+        expect(response.parsed_body["url"]).to eq(upload.url)
       end
 
       it 'returns cdn url' do