diff --git a/app/assets/javascripts/discourse/models/category.js b/app/assets/javascripts/discourse/models/category.js index 2a731e30cb0..d01e93110dc 100644 --- a/app/assets/javascripts/discourse/models/category.js +++ b/app/assets/javascripts/discourse/models/category.js @@ -224,7 +224,7 @@ Discourse.Category.reopenClass({ findSingleBySlug: function(slug) { return Discourse.Category.list().find(function(c) { - return Discourse.Category.slugFor(c) === slug || Discourse.Category.slugFor(c) === encodeURI(slug); + return Discourse.Category.slugFor(c) === slug; }); }, @@ -254,9 +254,7 @@ Discourse.Category.reopenClass({ if (slug === 'none') { return parentCategory; } category = categories.find(function(item) { - return item && item.get('parentCategory') === parentCategory && - (Discourse.Category.slugFor(item) === (parentSlug + "/" + slug) || - Discourse.Category.slugFor(item) === encodeURIComponent(parentSlug) + "/" + encodeURIComponent(slug)); + return item && item.get('parentCategory') === parentCategory && Discourse.Category.slugFor(item) === (parentSlug + "/" + slug); }); } } else { diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 358fdc5c529..523813c108e 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -427,8 +427,7 @@ class TopicsController < ApplicationController end def slugs_do_not_match - params[:slug] && (@topic_view.topic.slug != params[:slug] && - @topic_view.topic.slug != Rack::Utils.escape_path(params[:slug])) # maybe percent encoded + params[:slug] && @topic_view.topic.slug != params[:slug] end def redirect_to_correct_topic(topic, post_number=nil) diff --git a/app/models/category.rb b/app/models/category.rb index ea895cb025f..b5dca32e9af 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -207,7 +207,7 @@ SQL if slug.present? # santized custom slug - self.slug = Slug.for(Rack::Utils.unescape(slug), '') + self.slug = Slug.for(slug, '') errors.add(:slug, 'is already in use') if duplicate_slug? else # auto slug @@ -342,13 +342,11 @@ SQL def self.query_parent_category(parent_slug) self.where(slug: parent_slug, parent_category_id: nil).pluck(:id).first || - self.where(slug: Rack::Utils.escape_path(parent_slug), parent_category_id: nil).pluck(:id).first || self.where(id: parent_slug.to_i).pluck(:id).first end def self.query_category(slug_or_id, parent_category_id) self.where(slug: slug_or_id, parent_category_id: parent_category_id).includes(:featured_users).first || - self.where(slug: Rack::Utils.escape_path(slug_or_id), parent_category_id: parent_category_id).includes(:featured_users).first || self.where(id: slug_or_id.to_i, parent_category_id: parent_category_id).includes(:featured_users).first end diff --git a/app/models/topic.rb b/app/models/topic.rb index ef8d90d6573..81fafff804f 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -665,7 +665,7 @@ class Topic < ActiveRecord::Base def slug unless slug = read_attribute(:slug) return '' unless title.present? - slug = Slug.for(title, 'topic') + slug = Slug.for(title) if new_record? write_attribute(:slug, slug) else @@ -677,7 +677,7 @@ class Topic < ActiveRecord::Base end def title=(t) - slug = Slug.for(t.to_s, 'topic') + slug = Slug.for(t.to_s) write_attribute(:slug, slug) write_attribute(:title,t) end diff --git a/app/serializers/notification_serializer.rb b/app/serializers/notification_serializer.rb index 86ede10b919..50d0b1ef7b7 100644 --- a/app/serializers/notification_serializer.rb +++ b/app/serializers/notification_serializer.rb @@ -10,7 +10,7 @@ class NotificationSerializer < ApplicationSerializer :is_warning def slug - Slug.for(object.topic.title, 'topic') if object.topic.present? + Slug.for(object.topic.title) if object.topic.present? end def is_warning diff --git a/app/serializers/user_action_serializer.rb b/app/serializers/user_action_serializer.rb index 9fa5d292dd5..503aabe626d 100644 --- a/app/serializers/user_action_serializer.rb +++ b/app/serializers/user_action_serializer.rb @@ -58,7 +58,7 @@ class UserActionSerializer < ApplicationSerializer end def slug - Slug.for(object.title, 'topic') + Slug.for(object.title) end def include_slug? diff --git a/lib/onebox/engine/discourse_local_onebox.rb b/lib/onebox/engine/discourse_local_onebox.rb index 91d39e2f670..db4075b43e2 100644 --- a/lib/onebox/engine/discourse_local_onebox.rb +++ b/lib/onebox/engine/discourse_local_onebox.rb @@ -48,7 +48,7 @@ module Onebox return linked unless Guardian.new.can_see?(post) topic = post.topic - slug = Slug.for(topic.title, 'topic') + slug = Slug.for(topic.title) excerpt = post.excerpt(SiteSetting.post_onebox_maxlength) excerpt.gsub!("\n"," ") diff --git a/lib/slug.rb b/lib/slug.rb index afde72162d4..5ab91977149 100644 --- a/lib/slug.rb +++ b/lib/slug.rb @@ -7,7 +7,6 @@ module Slug when :ascii then self.ascii_generator(string) when :encoded then self.encoded_generator(string) when :none then self.none_generator(string) - else raise Discourse::SiteSettingMissing end # Reject slugs that only contain numbers, because they would be indistinguishable from id's. slug = (slug =~ /[^\d]/ ? slug : '') @@ -17,15 +16,20 @@ module Slug private def self.ascii_generator(string) - slug = string.gsub("'", "").parameterize - slug.gsub("_", "-") + string.gsub("'", "") + .parameterize + .gsub("_", "-") end def self.encoded_generator(string) - # strip and sanitized RFC 3986 reserved character and blank - string = string.strip.gsub(/\s+/, '-').gsub(/[:\/\?#\[\]@!\$\s&'\(\)\*\+,;=]+/, '') - string = Rack::Utils.escape_path(string) - string =~ /^-+$/ ? '' : string + # This generator will sanitize almost all special characters, + # including reserved characters from RFC3986. + # See also URI::REGEXP::PATTERN. + string.strip + .gsub(/\s+/, '-') + .gsub(/[:\/\?#\[\]@!\$&'\(\)\*\+,;=_\.~%\\`^\s|\{\}"<>]+/, '') # :/?#[]@!$&'()*+,;=_.~%\`^|{}"<> + .gsub(/\A-+|-+\z/, '') # remove possible trailing and preceding dashes + .squeeze('-') # squeeze continuous dashes to prettify slug end def self.none_generator(string) diff --git a/lib/tasks/build_test_topic.rake b/lib/tasks/build_test_topic.rake index 40d801d7c8f..e135cabc928 100644 --- a/lib/tasks/build_test_topic.rake +++ b/lib/tasks/build_test_topic.rake @@ -25,7 +25,7 @@ task 'build_test_topic' => :environment do first_post = PostCreator.new(evil_trout, raw: "This is the original post.", title: "pushState/replaceState test topic").create topic = first_post.topic - topic_url = "#{Discourse.base_url}/t/#{Slug.for(topic.title, 'topic')}/#{topic.id}" + topic_url = "#{Discourse.base_url}/t/#{Slug.for(topic.title)}/#{topic.id}" 99.times do |i| post_number = (i + 2) diff --git a/spec/components/slug_spec.rb b/spec/components/slug_spec.rb index 827adf82353..4173ab93288 100644 --- a/spec/components/slug_spec.rb +++ b/spec/components/slug_spec.rb @@ -10,15 +10,15 @@ describe Slug do before { SiteSetting.slug_generation_method = 'ascii' } it 'generates the slug' do - expect(Slug.for("hello world", 'topic')).to eq('hello-world') + expect(Slug.for("hello world")).to eq('hello-world') end it 'generates default slug when nothing' do - expect(Slug.for('', 'topic')).to eq('topic') + expect(Slug.for('')).to eq('topic') end it "doesn't generate slugs that are just numbers" do - expect(Slug.for('123', 'topic')).to eq('topic') + expect(Slug.for('123')).to eq('topic') end end @@ -27,15 +27,15 @@ describe Slug do after { SiteSetting.slug_generation_method = 'ascii' } it 'generates the slug' do - expect(Slug.for("熱帶風暴畫眉", 'topic')).to eq('%E7%86%B1%E5%B8%B6%E9%A2%A8%E6%9A%B4%E7%95%AB%E7%9C%89') + expect(Slug.for("熱帶風暴畫眉")).to eq('熱帶風暴畫眉') end it 'generates default slug when nothing' do - expect(Slug.for('', 'topic')).to eq('topic') + expect(Slug.for('')).to eq('topic') end it "doesn't generate slugs that are just numbers" do - expect(Slug.for('123', 'topic')).to eq('topic') + expect(Slug.for('123')).to eq('topic') end end @@ -45,9 +45,9 @@ describe Slug do it 'generates the slug' do expect(Slug.for("hello world", 'category')).to eq('category') - expect(Slug.for("hello world", 'topic')).to eq('topic') - expect(Slug.for('', 'topic')).to eq('topic') - expect(Slug.for('123', 'topic')).to eq('topic') + expect(Slug.for("hello world")).to eq('topic') + expect(Slug.for('')).to eq('topic') + expect(Slug.for('123')).to eq('topic') end end end @@ -111,14 +111,15 @@ describe Slug do after { SiteSetting.slug_generation_method = 'ascii' } it 'generates precentage encoded string' do - expect(Slug.encoded_generator("Jeff hate's !~-_|,=#this")).to eq("Jeff-hates-%7E-_%7Cthis") - expect(Slug.encoded_generator("뉴스피드")).to eq("%EB%89%B4%EC%8A%A4%ED%94%BC%EB%93%9C") - expect(Slug.encoded_generator("آموزش اضافه کردن لینک اختیاری به هدر")).to eq("%D8%A2%D9%85%D9%88%D8%B2%D8%B4-%D8%A7%D8%B6%D8%A7%D9%81%D9%87-%DA%A9%D8%B1%D8%AF%D9%86-%D9%84%DB%8C%D9%86%DA%A9-%D8%A7%D8%AE%D8%AA%DB%8C%D8%A7%D8%B1%DB%8C-%D8%A8%D9%87-%D9%87%D8%AF%D8%B1") - expect(Slug.encoded_generator("熱帶風暴畫眉")).to eq("%E7%86%B1%E5%B8%B6%E9%A2%A8%E6%9A%B4%E7%95%AB%E7%9C%89") + expect(Slug.encoded_generator("Jeff hate's !~-_|,=#this")).to eq("Jeff-hates-this") + expect(Slug.encoded_generator("뉴스피드")).to eq("뉴스피드") + expect(Slug.encoded_generator("آموزش اضافه کردن لینک اختیاری به هدر")).to eq("آموزش-اضافه-کردن-لینک-اختیاری-به-هدر") + expect(Slug.encoded_generator("熱帶風暴畫眉")).to eq("熱帶風暴畫眉") end it 'reject RFC 3986 reserved character and blank' do - expect(Slug.encoded_generator(":/?#[]@!$ &'()*+,;=")).to eq("") + expect(Slug.encoded_generator(":/?#[]@!$ &'()*+,;=% -_`~.")).to eq("") + expect(Slug.encoded_generator(" - English and Chinese title with special characters / 中文标题 !@:?\\:'`#^& $%&*()` -- ")).to eq("English-and-Chinese-title-with-special-characters-中文标题") end it 'generates null when nothing' do diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 33a56e2015b..19746499c64 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -209,8 +209,8 @@ describe Category do end it "creates a slug" do - expect(@category.slug).to eq("%E6%B5%8B%E8%AF%95") - expect(@category.slug_for_url).to eq("%E6%B5%8B%E8%AF%95") + expect(@category.slug).to eq("测试") + expect(@category.slug_for_url).to eq("测试") end end end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 105894c57bb..f1505448146 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -19,15 +19,15 @@ describe Topic do after { SiteSetting.slug_generation_method = 'ascii' } it "returns a Slug for a title" do - Slug.expects(:for).with(title, 'topic').returns(slug) + Slug.expects(:for).with(title).returns(slug) expect(Fabricate.build(:topic, title: title).slug).to eq(slug) end context 'for cjk characters' do let(:title) { "熱帶風暴畫眉" } - let(:slug) { "%E7%86%B1%E5%B8%B6%E9%A2%A8%E6%9A%B4%E7%95%AB%E7%9C%89" } + let(:slug) { "熱帶風暴畫眉" } it "returns encoded Slug for a title" do - Slug.expects(:for).with(title, 'topic').returns(slug) + Slug.expects(:for).with(title).returns(slug) expect(Fabricate.build(:topic, title: title).slug).to eq(slug) end end @@ -36,7 +36,7 @@ describe Topic do let(:title) { "123456789" } let(:slug) { "topic" } it 'generates default slug' do - Slug.expects(:for).with(title, 'topic').returns("topic") + Slug.expects(:for).with(title).returns("topic") expect(Fabricate.build(:topic, title: title).slug).to eq("topic") end end @@ -49,7 +49,7 @@ describe Topic do let(:slug) { "topic" } it "returns a Slug for a title" do - Slug.expects(:for).with(title, 'topic').returns('topic') + Slug.expects(:for).with(title).returns('topic') expect(Fabricate.build(:topic, title: title).slug).to eq(slug) end end @@ -57,7 +57,7 @@ describe Topic do context '#ascii_generator' do before { SiteSetting.slug_generation_method = 'ascii' } it "returns a Slug for a title" do - Slug.expects(:for).with(title, 'topic').returns(slug) + Slug.expects(:for).with(title).returns(slug) expect(Fabricate.build(:topic, title: title).slug).to eq(slug) end @@ -65,7 +65,7 @@ describe Topic do let(:title) { "熱帶風暴畫眉" } let(:slug) { 'topic' } it "returns 'topic' when the slug is empty (say, non-latin characters)" do - Slug.expects(:for).with(title, 'topic').returns("topic") + Slug.expects(:for).with(title).returns("topic") expect(Fabricate.build(:topic, title: title).slug).to eq("topic") end end