From 76ab0350f19e72cf33395768a8ce6736fec9565e Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Fri, 11 Oct 2019 12:38:16 -0300 Subject: [PATCH] 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. --- app/controllers/topics_controller.rb | 6 +++++- app/jobs/onceoff/fix_encoded_topic_slugs.rb | 14 ++++++++++++++ lib/slug.rb | 4 +++- spec/components/slug_spec.rb | 10 +++++----- spec/models/category_spec.rb | 4 ++-- spec/models/topic_spec.rb | 2 +- 6 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 app/jobs/onceoff/fix_encoded_topic_slugs.rb diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 9de9c79592d..d806413fbc0 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 00000000000..4640493fb9a --- /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 12fe660c436..8436f74ebce 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 d475e578c26..dba8bcb6f57 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 298bfc2b448..581d20b82ea 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 23dfdf5b75c..7674cc6fe97 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