From a781ef7662866942d3c0a17ddda59a656f514cb8 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Tue, 21 Apr 2020 03:50:50 +0200 Subject: [PATCH] FIX: Reject invalid Category slugs (#9473) Previously it would sanitize given slug and then save the resulting empty slug. --- app/models/category.rb | 7 ++++++- spec/requests/categories_controller_spec.rb | 3 ++- spec/requests/list_controller_spec.rb | 11 +++-------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index c19d3848cc1..192204d189f 100644 --- a/app/models/category.rb +++ b/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, '') diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index 91b1e4ebbb7..97293742d35 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/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 diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 83a46f33739..38daebea590 100644 --- a/spec/requests/list_controller_spec.rb +++ b/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| - DB.exec <<~SQL - UPDATE categories - SET slug = '#{category.id}-name' - WHERE id = #{c.id} - SQL - c.reload - } + Fabricate(:category_with_definition, name: "#{category.id} name", slug: 'will-be-changed').tap do |category| + category.update_column(:slug, "#{category.id}-name") + end } it 'uses the correct category' do