FIX: don't overwrite category's logo & background URLs

app/controllers/categories_controller.rb

@@ -132,7 +132,7 @@ class CategoriesController < ApplicationController
 
       render_serialized(@category, CategorySerializer)
     else
-      return render_json_error(@category) unless @category.save
+      render_json_error(@category) unless @category.save
     end
   end
 
@@ -151,6 +151,10 @@ class CategoriesController < ApplicationController
 
       old_permissions = cat.permissions_params
 
+      # remove asset host & cdn from both logo_url and background_url (never trust the client)
+      category_params[:logo_url] = fix_upload_url(category_params[:logo_url])
+      category_params[:background_url] = fix_upload_url(category_params[:background_url])
+
       if result = cat.update_attributes(category_params)
         Scheduler::Defer.later "Log staff action change category settings" do
           @staff_action_logger.log_category_settings_change(@category, category_params, old_permissions)
@@ -258,4 +262,24 @@ class CategoriesController < ApplicationController
       params[:include_topics] ||
       SiteSetting.desktop_category_page_style == "categories_with_featured_topics".freeze
     end
+
+    def fix_upload_url(url)
+      return if url.blank?
+
+      if Discourse.asset_host.present?
+        asset_host = UrlHelper.schemaless(Discourse.asset_host)
+        url.sub!(/^(https?:)?#{Regexp.escape(asset_host)}/, "")
+      end
+
+      if SiteSetting.enable_s3_uploads? && SiteSetting.s3_cdn_url.present?
+        s3_cdn_url = UrlHelper.schemaless(SiteSetting.s3_cdn_url)
+        url.sub!(/^(https?:)?#{Regexp.escape(s3_cdn_url)}/, Discourse.store.absolute_base_url)
+      end
+
+      base_url = UrlHelper.schemaless(Discourse.base_url_no_prefix)
+      url.sub!(/^(https?:)?#{Regexp.escape(base_url)}/, "")
+
+      url
+    end
+
 end

spec/controllers/categories_controller_spec.rb

@@ -230,6 +230,26 @@ describe CategoriesController do
 
           expect(UserHistory.count).to eq(5) # 2 + 3 (bootstrap mode)
         end
+
+        it "fix both logo and background urls" do
+          Discourse.stubs(:asset_host).returns("http://foo.bar")
+          Discourse.stubs(:base_url_no_prefix).returns("http://bar.foo")
+
+          upload = build(:upload)
+
+          xhr :put, :update, id: @category.id,
+                             name: @category.name,
+                             color: @category.color,
+                             text_color: @category.text_color,
+                             slug: @category.slug,
+                             logo_url: UrlHelper.schemaless(UrlHelper.absolute(upload.url)),
+                             background_url: UrlHelper.absolute_without_cdn(upload.url)
+
+          @category.reload
+          expect(@category.logo_url).to eq(upload.url)
+          expect(@category.background_url).to eq(upload.url)
+        end
+
       end
     end