FIX: Properly encoded slugs when configured to (#8158)

FIX: Properly encoded slugs when configured to (#8158)

When an admin changes the site setting slug_generation_method to encoded, we weren’t really encoding the slug, but just allowing non-ascii characters in the slug (unicode).

That brings problems when a user posts a link to topic without the slug, as our topic controller tries to redirect the user to the correct URL that contains the slug with unicode characters. Having unicode in the Location header in a response is a RFC violation and some browsers end up in a redirection loop.

Bug report: https://meta.discourse.org/t/-/125371?u=falco

This commit also checks if a site uses encoded slugs and clear all saved slugs in the db so they can be regenerated using an onceoff job.

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 9de9c79..d806413 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -900,7 +900,11 @@ class TopicsController < ApplicationController
   end
 
   def slugs_do_not_match
-    params[:slug] && @topic_view.topic.slug != params[:slug]
+    if SiteSetting.slug_generation_method != "encoded"
+      params[:slug] && @topic_view.topic.slug != params[:slug]
+    else
+      params[:slug] && CGI.unescape(@topic_view.topic.slug) != params[:slug]
+    end
   end
 
   def redirect_to_correct_topic(topic, post_number = nil)
diff --git a/app/jobs/onceoff/fix_encoded_topic_slugs.rb b/app/jobs/onceoff/fix_encoded_topic_slugs.rb
new file mode 100644
index 0000000..4640493
--- /dev/null
+++ b/app/jobs/onceoff/fix_encoded_topic_slugs.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module Jobs
+
+  class FixEncodedTopicSlugs < ::Jobs::Onceoff
+    def execute_onceoff(args)
+      return unless SiteSetting.slug_generation_method == 'encoded'
+
+      #Make all slugs nil and let the app regenerate with proper encoded ones
+      Topic.update_all(slug: nil)
+    end
+  end
+
+end
diff --git a/lib/slug.rb b/lib/slug.rb
index 12fe660..8436f74 100644
--- a/lib/slug.rb
+++ b/lib/slug.rb
@@ -50,7 +50,9 @@ module Slug
       .gsub(/\s+/, '-')
       .gsub(CHAR_FILTER_REGEXP, '')
 
-    downcase ? string.downcase : string
+    string = string.downcase if downcase
+
+    CGI.escape(string)
   end
 
   def self.none_generator(string)
diff --git a/spec/components/slug_spec.rb b/spec/components/slug_spec.rb
index d475e57..dba8bcb 100644
--- a/spec/components/slug_spec.rb
+++ b/spec/components/slug_spec.rb
@@ -60,7 +60,7 @@ describe Slug do
       after { SiteSetting.slug_generation_method = 'ascii' }
 
       it 'generates the slug' do
-        expect(Slug.for("熱帶風暴畫眉")).to eq('熱帶風暴畫眉')
+        expect(Slug.for("熱帶風暴畫眉")).to eq('%E7%86%B1%E5%B8%B6%E9%A2%A8%E6%9A%B4%E7%95%AB%E7%9C%89')
         expect(Slug.for("Jeff hate's !~-_|,=#this")).to eq("jeff-hates-this")
       end
 
@@ -75,7 +75,7 @@ describe Slug do
       it "handles the special characters" do
         expect(Slug.for(
           " - English and Chinese title with special characters / 中文标题 !@:?\\:'`#^& $%&*()` -- "
-        )).to eq("english-and-chinese-title-with-special-characters-中文标题")
+        )).to eq("english-and-chinese-title-with-special-characters-%E4%B8%AD%E6%96%87%E6%A0%87%E9%A2%98")
       end
 
       it "kills the trailing dash" do
@@ -151,9 +151,9 @@ describe Slug do
     after { SiteSetting.slug_generation_method = 'ascii' }
 
     it 'generates precentage encoded string' do
-      expect(Slug.encoded_generator("뉴스피드")).to eq("뉴스피드")
-      expect(Slug.encoded_generator("آموزش اضافه کردن لینک اختیاری به هدر")).to eq("آموزش-اضافه-کردن-لینک-اختیاری-به-هدر")
-      expect(Slug.encoded_generator("熱帶風暴畫眉")).to eq("熱帶風暴畫眉")
+      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")
     end
 
     it 'reject RFC 3986 reserved character and blank' do
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index 298bfc2..581d20b 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -313,8 +313,8 @@ describe Category do
       end
 
       it "creates a slug" do
-        expect(@category.slug).to eq("测试")
-        expect(@category.slug_for_url).to eq("测试")
+        expect(@category.slug).to eq("%E6%B5%8B%E8%AF%95")
+        expect(@category.slug_for_url).to eq("%E6%B5%8B%E8%AF%95")
       end
     end
   end
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 23dfdf5..7674cc6 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -158,7 +158,7 @@ describe Topic do
 
         it "returns encoded Slug for a title" do
           expect(topic.title).to eq(title)
-          expect(topic.slug).to eq(title)
+          expect(topic.slug).to eq('%E7%86%B1%E5%B8%B6%E9%A2%A8%E6%9A%B4%E7%95%AB%E7%9C%89')
         end
       end

GitHub sha: 76ab0350

1 Like