From 0e9451e93ff930a1d367e75c9a027387d5c8fa81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Thu, 16 May 2024 17:11:24 +0200
Subject: [PATCH] FIX: crawler view with unicode usernames (#27051)

When "unicode_usernames" is enabled, calling the "user_path" helper with a username containing some non ASCII character will break due to the route constraint we have on username.

This fixes the issue by always encoding the username before passing it to the "user_path" helper.

Internal ref - t/127547
---
 app/services/group_message.rb          | 5 ++++-
 app/views/about/index.html.erb         | 8 ++++----
 spec/requests/about_controller_spec.rb | 8 ++++++++
 spec/services/group_message_spec.rb    | 9 +++------
 4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/app/services/group_message.rb b/app/services/group_message.rb
index 47ee093a3fd..c55c9c66a9f 100644
--- a/app/services/group_message.rb
+++ b/app/services/group_message.rb
@@ -78,7 +78,10 @@ class GroupMessage
       begin
         h = { base_url: Discourse.base_url }.merge(@opts[:message_params] || {})
         if @opts[:user]
-          h.merge!(username: @opts[:user].username, user_url: user_path(@opts[:user].username))
+          h.merge!(
+            username: @opts[:user].username,
+            user_url: user_path(@opts[:user].encoded_username(lower: true)),
+          )
         end
         h
       end
diff --git a/app/views/about/index.html.erb b/app/views/about/index.html.erb
index 52bd1a5b9bb..a9219b05e4f 100644
--- a/app/views/about/index.html.erb
+++ b/app/views/about/index.html.erb
@@ -14,8 +14,8 @@
   <div class='admins-list' itemscope itemtype='http://schema.org/ItemList'>
     <% @about.admins.each do |user| %>
       <div itemprop='itemListElement' itemscope itemtype='http://schema.org/ListItem'>
-        <meta itemprop='url' content='<%= user_path(user.username_lower) %>'>
-        <a href='<%= user_path(user.username_lower) %>' itemprop='item'>
+        <meta itemprop='url' content='<%= user_path(user.encoded_username(lower: true)) %>'>
+        <a href='<%= user_path(user.encoded_username(lower: true)) %>' itemprop='item'>
           <span itemprop='image'>
             <img width="45" height="45" class="avatar" src="<%= user.small_avatar_url %>">
           </span>
@@ -35,8 +35,8 @@
     <div class='moderators-list' itemscope itemtype='http://schema.org/ItemList'>
       <% @about.moderators.each do |user| %>
         <div itemprop='itemListElement' itemscope itemtype='http://schema.org/ListItem'>
-          <meta itemprop='url' content='<%= user_path(user.username_lower) %>'>
-          <a href='<%= user_path(user.username_lower) %>' itemprop='item'>
+          <meta itemprop='url' content='<%= user_path(user.encoded_username(lower: true)) %>'>
+          <a href='<%= user_path(user.encoded_username(lower: true)) %>' itemprop='item'>
             <span itemprop='image'>
               <img width="45" height="45" class="avatar" src="<%= user.small_avatar_url %>">
             </span>
diff --git a/spec/requests/about_controller_spec.rb b/spec/requests/about_controller_spec.rb
index eb719ae9f7d..73bda70105b 100644
--- a/spec/requests/about_controller_spec.rb
+++ b/spec/requests/about_controller_spec.rb
@@ -38,6 +38,14 @@ RSpec.describe AboutController do
         expect(response.status).to eq(200)
         expect(response.body).to include("/u/anadminuser")
       end
+
+      it "supports unicode usernames" do
+        SiteSetting.unicode_usernames = true
+        Fabricate(:admin, username: "martínez")
+        get "/about", headers: { "HTTP_USER_AGENT" => "Googlebot" }
+        expect(response.status).to eq(200)
+        expect(response.body).to include("/u/mart%25C3%25ADnez")
+      end
     end
 
     it "serializes stats when 'Guardian#can_see_about_stats?' is true" do
diff --git a/spec/services/group_message_spec.rb b/spec/services/group_message_spec.rb
index 4cd007becfc..76a4011ddb8 100644
--- a/spec/services/group_message_spec.rb
+++ b/spec/services/group_message_spec.rb
@@ -7,8 +7,8 @@ RSpec.describe GroupMessage do
 
   let(:moderators_group) { Group[:moderators].name }
 
-  let!(:admin) { Fabricate.build(:admin, id: 999) }
-  let!(:user) { Fabricate.build(:user, id: 111) }
+  fab!(:admin)
+  fab!(:user)
 
   before { Discourse.stubs(:system_user).returns(admin) }
 
@@ -57,11 +57,10 @@ RSpec.describe GroupMessage do
   end
 
   describe "message_params" do
-    let(:user) { Fabricate.build(:user, id: 123_123) }
     shared_examples "common message params for group messages" do
       it "returns the correct params" do
         expect(message_params[:username]).to eq(user.username)
-        expect(message_params[:user_url]).to be_present
+        expect(message_params[:user_url]).to eq("/u/#{user.username}")
       end
     end
 
@@ -87,8 +86,6 @@ RSpec.describe GroupMessage do
       GroupMessage.new(moderators_group, :user_automatically_silenced, user: user)
     end
 
-    let(:user) { Fabricate.build(:user, id: 123_123) }
-
     before do
       PostCreator.stubs(:create).returns(stub_everything)
       group_message.stubs(:sent_recently_key).returns("the_key")