Merge branch 'master' of github.com:discourse/discourse

Gemfile

@@ -43,6 +43,13 @@ gem 'mini_mime'
 gem 'mini_suffix'
 
 gem 'redis'
+
+# This is explicitly used by Sidekiq and is an optional dependency.
+# We tell Sidekiq to use the namespace "sidekiq" which triggers this
+# gem to be used. There is no explicit dependency in sidekiq cause
+# redis namespace support is optional
+# We already namespace stuff in DiscourseRedis, so we should consider
+# just using a single implementation in core vs having 2 namespace implementations
 gem 'redis-namespace'
 
 # NOTE: AM serializer gets a lot slower with recent updates
@@ -127,6 +134,7 @@ gem 'highline', '~> 1.7.0', require: false
 gem 'rack-protection' # security
 gem 'cbor', require: false
 gem 'cose', require: false
+gem 'addressable', '~> 2.7.0'
 
 # Gems used only for assets and not required in production environments by default.
 # Allow everywhere for now cause we are allowing asset debugging in production

Gemfile.lock

@@ -272,7 +272,7 @@ GEM
       nio4r (~> 2.0)
     r2 (0.2.7)
     rack (2.0.7)
-    rack-mini-profiler (1.1.3)
+    rack-mini-profiler (1.1.4)
       rack (>= 1.2.0)
     rack-openid (1.3.1)
       rack (>= 1.1.0)
@@ -309,7 +309,7 @@ GEM
       optimist (>= 3.0.0)
     rchardet (1.8.0)
     redis (4.1.3)
-    redis-namespace (1.6.0)
+    redis-namespace (1.7.0)
       redis (>= 3.0.4)
     request_store (1.4.1)
       rack (>= 1.4)
@@ -398,7 +398,7 @@ GEM
       activesupport (>= 4.0)
       sprockets (>= 3.0.0)
     sshkey (2.0.0)
-    stackprof (0.2.13)
+    stackprof (0.2.14)
     test-prof (0.10.1)
     thor (0.20.3)
     thread_safe (0.3.6)
@@ -437,6 +437,7 @@ DEPENDENCIES
   activemodel (= 6.0.1)
   activerecord (= 6.0.1)
   activesupport (= 6.0.1)
+  addressable (~> 2.7.0)
   annotate
   aws-sdk-s3
   aws-sdk-sns

app/controllers/list_controller.rb

@@ -304,7 +304,7 @@ class ListController < ApplicationController
         (slug_path + [@category.id.to_s]).join("/")
     end
 
-    route_params[:username] = UrlHelper.escape_uri(params[:username]) if params[:username].present?
+    route_params[:username] = UrlHelper.encode_component(params[:username]) if params[:username].present?
     route_params
   end
 
@@ -374,7 +374,7 @@ class ListController < ApplicationController
 
     opts = opts.dup
     if SiteSetting.unicode_usernames && opts[:group_name]
-      opts[:group_name] = URI.encode(opts[:group_name])
+      opts[:group_name] = UrlHelper.encode_component(opts[:group_name])
     end
     opts.delete(:category) if page_params.include?(:category_slug_path_with_id)
 

app/controllers/users_controller.rb

@@ -60,8 +60,6 @@ class UsersController < ApplicationController
     user_serializer = nil
     if guardian.can_see_profile?(@user)
       user_serializer = UserSerializer.new(@user, scope: guardian, root: 'user')
-      # TODO remove this options from serializer
-      user_serializer.omit_stats = true
 
       topic_id = params[:include_post_count_for].to_i
       if topic_id != 0

app/helpers/application_helper.rb

@@ -383,8 +383,7 @@ module ApplicationHelper
 
   def topic_featured_link_domain(link)
     begin
-      uri = URI.encode(link)
-      uri = URI.parse(uri)
+      uri = UrlHelper.encode_and_parse(link)
       uri = URI.parse("http://#{uri}") if uri.scheme.nil?
       host = uri.host.downcase
       host.start_with?('www.') ? host[4..-1] : host

app/jobs/regular/download_backup_email.rb

@@ -15,7 +15,7 @@ module Jobs
       raise Discourse::InvalidParameters.new(:backup_file_path) if backup_file_path.blank?
 
       backup_file_path = URI(backup_file_path)
-      backup_file_path.query = URI.encode_www_form(token: EmailBackupToken.set(user.id))
+      backup_file_path.query = { token: EmailBackupToken.set(user.id) }.to_param
 
       message = DownloadBackupMailer.send_email(user.email, backup_file_path.to_s)
       Email::Sender.new(message, :download_backup_message).send

app/jobs/regular/update_username.rb

@@ -27,7 +27,7 @@ module Jobs
 
       cooked_username = PrettyText::Helpers.format_username(@old_username)
       @cooked_mention_username_regex = /^@#{cooked_username}$/i
-      @cooked_mention_user_path_regex = /^\/u(?:sers)?\/#{CGI.escape(cooked_username)}$/i
+      @cooked_mention_user_path_regex = /^\/u(?:sers)?\/#{UrlHelper.encode_component(cooked_username)}$/i
       @cooked_quote_username_regex = /(?<=\s)#{cooked_username}(?=:)/i
 
       update_posts

app/mailers/user_notifications.rb

@@ -624,7 +624,7 @@ class UserNotifications < ActionMailer::Base
 
     email_opts = {
       topic_title: Emoji.gsub_emoji_to_unicode(title),
-      topic_title_url_encoded: title ? URI.encode(title) : title,
+      topic_title_url_encoded: title ? UrlHelper.encode_component(title) : title,
       message: message,
       url: post.url(without_slug: SiteSetting.private_email?),
       post_id: post.id,
@@ -649,7 +649,7 @@ class UserNotifications < ActionMailer::Base
       use_topic_title_subject: use_topic_title_subject,
       site_description: SiteSetting.site_description,
       site_title: SiteSetting.title,
-      site_title_url_encoded: URI.encode(SiteSetting.title),
+      site_title_url_encoded: UrlHelper.encode_component(SiteSetting.title),
       locale: locale
     }
 

app/models/concerns/has_url.rb

@@ -22,7 +22,7 @@ module HasUrl
       return if url.blank?
 
       uri = begin
-        URI(URI.unescape(url))
+        URI(UrlHelper.unencode(url))
       rescue URI::Error
       end
 

app/models/embeddable_host.rb

@@ -34,7 +34,7 @@ class EmbeddableHost < ActiveRecord::Base
       return eh if eh.path_whitelist.blank?
 
       path_regexp = Regexp.new(eh.path_whitelist)
-      return eh if path_regexp.match(path) || path_regexp.match(URI.unescape(path))
+      return eh if path_regexp.match(path) || path_regexp.match(UrlHelper.unencode(path))
     end
 
     nil

app/models/post.rb

@@ -971,7 +971,7 @@ class Post < ActiveRecord::Base
       next unless Discourse.store.has_been_uploaded?(src) || (include_local_upload && src =~ /\A\/[^\/]/i)
 
       path = begin
-        URI(URI.unescape(GlobalSetting.cdn_url ? src.sub(GlobalSetting.cdn_url, "") : src))&.path
+        URI(UrlHelper.unencode(GlobalSetting.cdn_url ? src.sub(GlobalSetting.cdn_url, "") : src))&.path
       rescue URI::Error
       end
 

app/models/topic.rb

@@ -1354,7 +1354,7 @@ class Topic < ActiveRecord::Base
   end
 
   def featured_link_root_domain
-    MiniSuffix.domain(URI.parse(URI.encode(self.featured_link)).hostname)
+    MiniSuffix.domain(UrlHelper.encode_and_parse(self.featured_link).hostname)
   end
 
   def self.private_message_topics_count_per_day(start_date, end_date, topic_subtype)

app/models/user.rb

@@ -768,8 +768,8 @@ class User < ActiveRecord::Base
       url = SiteSetting.external_system_avatars_url.dup
       url = +"#{Discourse::base_uri}#{url}" unless url =~ /^https?:\/\//
       url.gsub! "{color}", letter_avatar_color(normalized_username)
-      url.gsub! "{username}", CGI.escape(username)
-      url.gsub! "{first_letter}", CGI.escape(normalized_username.grapheme_clusters.first)
+      url.gsub! "{username}", UrlHelper.encode_component(username)
+      url.gsub! "{first_letter}", UrlHelper.encode_component(normalized_username.grapheme_clusters.first)
       url.gsub! "{hostname}", Discourse.current_hostname
       url
     else

app/serializers/user_serializer.rb

@@ -2,8 +2,7 @@
 
 class UserSerializer < BasicUserSerializer
 
-  attr_accessor :omit_stats,
-                :topic_post_count
+  attr_accessor :topic_post_count
 
   def self.staff_attributes(*attrs)
     attributes(*attrs)
@@ -48,7 +47,6 @@ class UserSerializer < BasicUserSerializer
              :can_edit_username,
              :can_edit_email,
              :can_edit_name,
-             :stats,
              :ignored,
              :muted,
              :can_ignore_user,
@@ -109,7 +107,6 @@ class UserSerializer < BasicUserSerializer
                      :tracked_category_ids,
                      :watched_category_ids,
                      :watched_first_post_category_ids,
-                     :private_messages_stats,
                      :system_avatar_upload_id,
                      :system_avatar_template,
                      :gravatar_avatar_upload_id,
@@ -257,14 +254,6 @@ class UserSerializer < BasicUserSerializer
     scope.can_edit_name?(object)
   end
 
-  def include_stats?
-    !omit_stats == true
-  end
-
-  def stats
-    UserAction.stats(object.id, scope)
-  end
-
   def ignored
     IgnoredUser.where(user_id: scope.user&.id, ignored_user_id: object.id).exists?
   end
@@ -378,14 +367,6 @@ class UserSerializer < BasicUserSerializer
     IgnoredUser.where(user_id: object.id).joins(:ignored_user).pluck(:username)
   end
 
-  def include_private_messages_stats?
-    can_edit && !(omit_stats == true)
-  end
-
-  def private_messages_stats
-    UserAction.private_messages_stats(object.id, scope)
-  end
-
   def system_avatar_upload_id
     # should be left blank
   end

app/serializers/web_hook_user_serializer.rb

@@ -3,10 +3,6 @@
 class WebHookUserSerializer < UserSerializer
   attributes :external_id
 
-  def omit_stats
-    true
-  end
-
   # remove staff attributes
   def staff_attributes(*attrs)
   end

lib/final_destination.rb

@@ -315,10 +315,7 @@ class FinalDestination
   end
 
   def escape_url
-    UrlHelper.escape_uri(
-      CGI.unescapeHTML(@url),
-      Regexp.new("[^#{URI::PATTERN::UNRESERVED}#{URI::PATTERN::RESERVED}#]")
-    )
+    UrlHelper.escape_uri(@url)
   end
 
   def private_ranges

lib/url_helper.rb

@@ -10,13 +10,31 @@ class UrlHelper
     url, fragment = url.split("#", 2)
     uri = URI.parse(url)
     if uri
-      fragment = URI.escape(fragment) if fragment&.include?('#')
+      # Addressable::URI::CharacterClasses::UNRESERVED is used here because without it
+      # the # in the fragment is not encoded
+      fragment = Addressable::URI.encode_component(fragment, Addressable::URI::CharacterClasses::UNRESERVED) if fragment&.include?('#')
       uri.fragment = fragment
       uri
     end
   rescue URI::Error
   end
 
+  def self.encode_and_parse(url)
+    URI.parse(Addressable::URI.encode(url))
+  end
+
+  def self.encode(url)
+    Addressable::URI.encode(url)
+  end
+
+  def self.unencode(url)
+    Addressable::URI.unencode(url)
+  end
+
+  def self.encode_component(url_component)
+    Addressable::URI.encode_component(url_component)
+  end
+
   def self.is_local(url)
     url.present? && (
       Discourse.store.has_been_uploaded?(url) ||
@@ -43,14 +61,10 @@ class UrlHelper
     self.absolute(url, nil)
   end
 
-  DOUBLE_ESCAPED_REGEXP ||= /%25([0-9a-f]{2})/i
-
   # Prevents double URL encode
   # https://stackoverflow.com/a/37599235
-  def self.escape_uri(uri, pattern = URI::UNSAFE)
-    encoded = URI.encode(uri, pattern)
-    encoded.gsub!(DOUBLE_ESCAPED_REGEXP, '%\1')
-    encoded
+  def self.escape_uri(uri)
+    UrlHelper.encode_component(CGI.unescapeHTML(UrlHelper.unencode(uri)))
   end
 
   def self.cook_url(url, secure: false)

lib/validators/url_validator.rb

@@ -9,7 +9,7 @@ class UrlValidator < ActiveModel::EachValidator
           uri.is_a?(URI::HTTP) && !uri.host.nil? && uri.host.include?(".")
         rescue URI::Error => e
           if (e.message =~ /URI must be ascii only/)
-            value = URI.encode(value)
+            value = UrlHelper.encode(value)
             retry
           end
 

script/import_scripts/ipboard.rb

@@ -972,7 +972,7 @@ EOM
     User.find_each do |u|
       ucf = u.custom_fields
       if ucf && ucf["import_id"] && ucf["import_username"]
-        username = URI.escape(ucf["import_username"])
+        username = UrlHelper.encode_component(ucf["import_username"])
         Permalink.create(url: "#{USERDIR}/#{ucf['import_id']}-#{username}", external_url: "/users/#{u.username}") rescue nil
         print '.'
       end

spec/requests/list_controller_spec.rb

@@ -181,7 +181,7 @@ RSpec.describe ListController do
         unicode_group = Fabricate(:group, name: '群群组')
         unicode_group.add(user)
         topic = Fabricate(:private_message_topic, allowed_groups: [unicode_group])
-        get "/topics/private-messages-group/#{user.username}/#{URI.escape(unicode_group.name)}.json"
+        get "/topics/private-messages-group/#{user.username}/#{UrlHelper.encode_component(unicode_group.name)}.json"
         expect(response.status).to eq(200)
 
         expect(JSON.parse(response.body)["topic_list"]["topics"].first["id"])