store the slug as the title is, only sanitize the slug

and prettify code

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 {

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)

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
 

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

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

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?

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"," ")

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)

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)

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

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

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