From 6e9c8fe8543ccfff83029e8440136da08f83865f Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Wed, 16 Oct 2019 17:08:43 -0300 Subject: [PATCH] FIX: More encoded slug fixes (#8191) * FIX: Do not encode the URL twice Now that we encode slugs in the server we don't need this anymore. Reverts fe5na33 * FIX: More places do deal with encoded slugs * the param is a string now, not a hash * FIX: Handle the nil slug on /categories * DEV: Add seeded? method to identity default categories * DEV: Use SiteSetting to keep track of seeded categories --- .../discourse/components/share-popup.js.es6 | 2 +- .../discourse/models/category.js.es6 | 6 +++++- app/controllers/categories_controller.rb | 2 +- app/controllers/topics_controller.rb | 6 +++--- .../onceoff/fix_encoded_category_slugs.rb | 17 ++++++++++++++++ app/models/category.rb | 20 ++++++++++++++++++- app/models/topic.rb | 9 +++++++++ config/site_settings.yml | 1 + 8 files changed, 56 insertions(+), 7 deletions(-) create mode 100644 app/jobs/onceoff/fix_encoded_category_slugs.rb diff --git a/app/assets/javascripts/discourse/components/share-popup.js.es6 b/app/assets/javascripts/discourse/components/share-popup.js.es6 index dad5d68949f..10e723a196a 100644 --- a/app/assets/javascripts/discourse/components/share-popup.js.es6 +++ b/app/assets/javascripts/discourse/components/share-popup.js.es6 @@ -85,7 +85,7 @@ export default Ember.Component.extend({ if (!this.site.mobileView) { $this.css({ left: "" + x + "px" }); } - this.set("link", encodeURI(url)); + this.set("link", url); this.set("visible", true); Ember.run.scheduleOnce("afterRender", this, this._focusUrl); diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6 index 8f53b70f5d3..11bdbf9b8a3 100644 --- a/app/assets/javascripts/discourse/models/category.js.es6 +++ b/app/assets/javascripts/discourse/models/category.js.es6 @@ -243,7 +243,11 @@ Category.reopenClass({ }, findSingleBySlug(slug) { - return Category.list().find(c => Category.slugFor(c) === slug); + if (Discourse.SiteSettings.slug_generation_method !== "encoded") { + return Category.list().find(c => Category.slugFor(c) === slug); + } else { + return Category.list().find(c => Category.slugFor(c) === encodeURI(slug)); + } }, findById(id) { diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index a548e6c815a..c20c08bcac1 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -18,7 +18,7 @@ class CategoriesController < ApplicationController @description = SiteSetting.site_description - parent_category = Category.find_by(slug: params[:parent_category_id]) || Category.find_by(id: params[:parent_category_id].to_i) + parent_category = Category.find_by_slug(params[:parent_category_id]) || Category.find_by(id: params[:parent_category_id].to_i) category_options = { is_homepage: current_homepage == "categories".freeze, diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index d806413fbc0..1b2bc12702e 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -36,7 +36,7 @@ class TopicsController < ApplicationController skip_before_action :check_xhr, only: [:show, :feed] def id_for_slug - topic = Topic.find_by(slug: params[:slug].downcase) + topic = Topic.find_by_slug(params[:slug]) guardian.ensure_can_see!(topic) raise Discourse::NotFound unless topic render json: { slug: topic.slug, topic_id: topic.id, url: topic.url } @@ -64,7 +64,7 @@ class TopicsController < ApplicationController # Special case: a slug with a number in front should look by slug first before looking # up that particular number if params[:id] && params[:id] =~ /^\d+[^\d\\]+$/ - topic = Topic.find_by(slug: params[:id].downcase) + topic = Topic.find_by_slug(params[:id]) return redirect_to_correct_topic(topic, opts[:post_number]) if topic end @@ -81,7 +81,7 @@ class TopicsController < ApplicationController @topic_view = TopicView.new(params[:id] || params[:topic_id], current_user, opts) rescue Discourse::NotFound => ex if params[:id] - topic = Topic.find_by(slug: params[:id].downcase) + topic = Topic.find_by_slug(params[:id]) return redirect_to_correct_topic(topic, opts[:post_number]) if topic end diff --git a/app/jobs/onceoff/fix_encoded_category_slugs.rb b/app/jobs/onceoff/fix_encoded_category_slugs.rb new file mode 100644 index 00000000000..28947125d90 --- /dev/null +++ b/app/jobs/onceoff/fix_encoded_category_slugs.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Jobs + + class FixEncodedCategorySlugs < ::Jobs::Onceoff + def execute_onceoff(args) + return unless SiteSetting.slug_generation_method == 'encoded' + + #Make custom categories slugs nil and let the app regenerate with proper encoded ones + Category.all.reject { |c| c.seeded? }.each do |c| + c.slug = nil + c.save! + end + end + end + +end diff --git a/app/models/category.rb b/app/models/category.rb index 2dcdd7d1e86..e76089baad3 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -612,7 +612,8 @@ class Category < ActiveRecord::Base end def self.query_category(slug_or_id, parent_category_id) - self.where(slug: slug_or_id, parent_category_id: parent_category_id).first || + encoded_slug_or_id = CGI.escape(slug_or_id) if SiteSetting.slug_generation_method == 'encoded' + self.where(slug: (encoded_slug_or_id || slug_or_id), parent_category_id: parent_category_id).first || self.where(id: slug_or_id.to_i, parent_category_id: parent_category_id).first end @@ -629,6 +630,15 @@ class Category < ActiveRecord::Base id == SiteSetting.uncategorized_category_id end + def seeded? + [ + SiteSetting.lounge_category_id, + SiteSetting.meta_category_id, + SiteSetting.staff_category_id, + SiteSetting.uncategorized_category_id, + ].include? id + end + @@url_cache = DistributedCache.new('category_url') def clear_url_cache @@ -708,6 +718,14 @@ class Category < ActiveRecord::Base end def self.find_by_slug(category_slug, parent_category_slug = nil) + + return nil if category_slug.nil? + + if SiteSetting.slug_generation_method == "encoded" + parent_category_slug = CGI.escape(parent_category_slug) unless parent_category_slug.nil? + category_slug = CGI.escape(category_slug) + end + if parent_category_slug parent_category_id = self.where(slug: parent_category_slug, parent_category_id: nil).select(:id) diff --git a/app/models/topic.rb b/app/models/topic.rb index ed98ba47f38..9061198bab4 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -988,6 +988,15 @@ class Topic < ActiveRecord::Base slug end + def self.find_by_slug(slug) + if SiteSetting.slug_generation_method != "encoded" + Topic.find_by(slug: slug.downcase) + else + encoded_slug = CGI.escape(slug) + Topic.find_by(slug: encoded_slug) + end + end + def title=(t) slug = Slug.for(t.to_s) write_attribute(:slug, slug) diff --git a/config/site_settings.yml b/config/site_settings.yml index 2f3962e4c01..603ac3cc897 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1730,6 +1730,7 @@ uncategorized: slug_generation_method: default: "ascii" enum: "SlugSetting" + client: true locale_default: ja: "none" zh_CN: "none"