FIX: Reject invalid Category slugs (#9473)

Previously it would sanitize given slug and then save the resulting empty slug.

app/models/category.rb

@@ -343,7 +343,12 @@ class Category < ActiveRecord::Base
       slug = SiteSetting.slug_generation_method == 'encoded' ? CGI.unescape(self.slug) : self.slug
       # sanitize the custom slug
       self.slug = Slug.sanitize(slug)
-      errors.add(:slug, 'is already in use') if duplicate_slug?
+
+      if self.slug.blank?
+        errors.add(:slug, :invalid)
+      elsif duplicate_slug?
+        errors.add(:slug, 'is already in use')
+      end
     else
       # auto slug
       self.slug = Slug.for(name, '')

spec/requests/categories_controller_spec.rb

@@ -481,8 +481,9 @@ describe CategoriesController do
       end
 
       it 'rejects invalid custom slug' do
-        put "/category/#{category.id}/slug.json", params: { slug: '  ' }
+        put "/category/#{category.id}/slug.json", params: { slug: '.' }
         expect(response.status).to eq(422)
+        expect(response.parsed_body["errors"]).to eq(["Slug is invalid"])
       end
     end
   end

spec/requests/list_controller_spec.rb

@@ -376,14 +376,9 @@ RSpec.describe ListController do
         # One category has another category's id at the beginning of its name
         let!(:other_category) {
           # Our validations don't allow this to happen now, but did historically
-          Fabricate(:category_with_definition, name: "#{category.id} name", slug: '-').tap { |c|
+          Fabricate(:category_with_definition, name: "#{category.id} name", slug: 'will-be-changed').tap do |category|
-            DB.exec <<~SQL
+            category.update_column(:slug, "#{category.id}-name")
-              UPDATE categories
+          end
-              SET slug = '#{category.id}-name'
-              WHERE id = #{c.id}
-            SQL
-            c.reload
-          }
         }
 
         it 'uses the correct category' do