From bcc42a130d4eb77b10b78b349d7c82583befcb2b Mon Sep 17 00:00:00 2001
From: Jamie Gaskins <jamie@forem.com>
Date: Tue, 11 May 2021 11:21:03 -0400
Subject: [PATCH] Sanitize tag urls (#13720)

* Sanitize tag URLs

For tags with non-ASCII names, we were generating busted URLs, which
would then cause errors. This commit fixes that particular error by
generating usable URLs.

* Use URL generator that will sanitize tag names

* Remove path prefix when using a URL generator
---
 app/lib/url.rb                               | 2 +-
 app/views/admin/tags/edit.html.erb           | 2 +-
 app/views/articles/_single_story.html.erb    | 2 +-
 app/views/dashboards/following_tags.html.erb | 2 +-
 app/views/tags/_liquid.html.erb              | 2 +-
 app/views/tags/edit.html.erb                 | 2 +-
 app/views/tags/index.html.erb                | 2 +-
 spec/lib/url_spec.rb                         | 7 +++++++
 8 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/app/lib/url.rb b/app/lib/url.rb
index f8d21106c..4d00db689 100644
--- a/app/lib/url.rb
+++ b/app/lib/url.rb
@@ -46,7 +46,7 @@ module URL
   #
   # @param tag [Tag] the tag to create the URL for
   def self.tag(tag, page = 1)
-    url(["/t/#{tag.name}", ("/page/#{page}" if page > 1)].join)
+    url(["/t/#{CGI.escape(tag.name)}", ("/page/#{page}" if page > 1)].join)
   end
 
   # Creates a user URL
diff --git a/app/views/admin/tags/edit.html.erb b/app/views/admin/tags/edit.html.erb
index 2f0eb7fa3..082d2aa25 100644
--- a/app/views/admin/tags/edit.html.erb
+++ b/app/views/admin/tags/edit.html.erb
@@ -5,7 +5,7 @@
   </div>
 
   <div class="ml-auto">
-    <a href="<%= tag_path(@tag.name) %>" class="crayons-btn crayons-btn--outlined" target="_blank" rel="noopener noreferer">View</a>
+    <a href="<%= URL.tag(@tag) %>" class="crayons-btn crayons-btn--outlined" target="_blank" rel="noopener noreferer">View</a>
   </div>
 </header>
 
diff --git a/app/views/articles/_single_story.html.erb b/app/views/articles/_single_story.html.erb
index cf5569bb3..be8dcdc50 100644
--- a/app/views/articles/_single_story.html.erb
+++ b/app/views/articles/_single_story.html.erb
@@ -59,7 +59,7 @@
       <div class="crayons-story__tags">
         <% flare_tag = FlareTag.new(story, @tag).tag %>
         <% if flare_tag %>
-          <a href="/t/<%= flare_tag.name %>" class="crayons-tag" style="background:<%= flare_tag.bg_color_hex %>;color:<%= flare_tag.text_color_hex %>"><span class="crayons-tag__prefix">#</span><%= flare_tag.name %></a>
+          <a href="<%= URL.tag flare_tag.name %>" class="crayons-tag" style="background:<%= flare_tag.bg_color_hex %>;color:<%= flare_tag.text_color_hex %>"><span class="crayons-tag__prefix">#</span><%= flare_tag.name %></a>
         <% end %>
         <% tags_to_display = story.cached_tag_list_array
            if flare_tag
diff --git a/app/views/dashboards/following_tags.html.erb b/app/views/dashboards/following_tags.html.erb
index cb9086f51..11bf1bae7 100644
--- a/app/views/dashboards/following_tags.html.erb
+++ b/app/views/dashboards/following_tags.html.erb
@@ -30,7 +30,7 @@
                 <% color = Color::CompareHex.new([tag.bg_color_hex || "#0000000", tag.text_color_hex || "#ffffff"]).brightness(0.8) %>
                 <div class="crayons-card branded-2 p-4 m:p-6 m:pt-4 flex flex-col single-article break-word content-center <% if follow.explicit_points < 0 %>opacity-75<% end %>" style="border-top-color: <%= color %>;" id="follows-<%= follow.id %>">
                   <h3 class="s:mb-1 -ml-1 p-0 fw-medium">
-                    <a href="/t/<%= tag.name %>" class="crayons-tag crayons-tag--l">
+                    <a href="<%= URL.tag tag %>" class="crayons-tag crayons-tag--l">
                       <span class="crayons-tag__prefix">#</span><%= tag.name %>
                     </a>
                     <% if follow.explicit_points < 0 %>
diff --git a/app/views/tags/_liquid.html.erb b/app/views/tags/_liquid.html.erb
index 1999cc821..7a3736208 100644
--- a/app/views/tags/_liquid.html.erb
+++ b/app/views/tags/_liquid.html.erb
@@ -7,7 +7,7 @@
     }
   </style>
     <div class="ltag__tag__content">
-      <h2>#<a href="/t/<%= tag.name %>" class="ltag__tag__link"><%= tag.name %></a> <%= follow_btn %></h2>
+      <h2>#<a href="<%= URL.tag tag %>" class="ltag__tag__link"><%= tag.name %></a> <%= follow_btn %></h2>
       <div class="ltag__tag__summary">
         <%= tag.short_summary %>
       </div>
diff --git a/app/views/tags/edit.html.erb b/app/views/tags/edit.html.erb
index 514c6984f..9b1ad729e 100644
--- a/app/views/tags/edit.html.erb
+++ b/app/views/tags/edit.html.erb
@@ -21,7 +21,7 @@
   <div class="slanty-accent"></div>
   <div class="tag-or-query-header-container tag-edit-tag">
     <h1> Editing:
-      <a href="/t/<%= @tag.name %>" style="color: <%= @tag.text_color_hex %>;text-decoration: underline;"><%= @tag.name %></a>
+      <a href="<%= URL.tag @tag %>" style="color: <%= @tag.text_color_hex %>;text-decoration: underline;"><%= @tag.name %></a>
     </h1>
     <p>
       <code style="background-color: #D2D2D2; color: black; padding: 5px; border-radius: 5px;">#<%= @tag.name %></code> is the name of the tag used in Markdown.
diff --git a/app/views/tags/index.html.erb b/app/views/tags/index.html.erb
index 86547cdcc..e04b83c8b 100644
--- a/app/views/tags/index.html.erb
+++ b/app/views/tags/index.html.erb
@@ -29,7 +29,7 @@
       <% color = Color::CompareHex.new([tag.bg_color_hex || "#0000000", tag.text_color_hex || "#ffffff"]).brightness(0.8) %>
       <div class="tag-card crayons-card branded-4 p-4 m:p-6 m:pt-4 flex flex-col relative" style="border-top-color: <%= color %>; ">
         <h3 class="crayons-tag crayons-tag--l mb-2">
-          <a href="/t/<%= tag.name %>" class="crayons-link">
+          <a href="<%= URL.tag tag %>" class="crayons-link">
             <span class="crayons-tag__prefix">#</span><%= tag.name %>
           </a>
         </h3>
diff --git a/spec/lib/url_spec.rb b/spec/lib/url_spec.rb
index 797d0b329..d67ca75d7 100644
--- a/spec/lib/url_spec.rb
+++ b/spec/lib/url_spec.rb
@@ -94,6 +94,13 @@ RSpec.describe URL, type: :lib do
     it "returns the correct URL for a tag" do
       expect(described_class.tag(tag, 2)).to eq("https://dev.to/t/#{tag.name}/page/2")
     end
+
+    it "returns the correct URL for a tag with a non-ASCII name" do
+      # Due to validations we can't trivially create a tag with a non-ASCII name
+      # so we just create something that quacks like one.
+      tag = instance_double("Tag", name: "histórico")
+      expect(described_class.tag(tag)).to eq("https://dev.to/t/hist%C3%B3rico")
+    end
   end
 
   describe ".deep_link" do