From 315f1ec4cfd7fa2adee3fb95955c150afbc71861 Mon Sep 17 00:00:00 2001
From: Khadija Sidhpuri <59063821+squarebat@users.noreply.github.com>
Date: Thu, 15 Jul 2021 22:29:29 +0530
Subject: [PATCH] Fix shorten_urls to not strip out text formatting in comments
 (#14240)

* fix: shorten_urls to only shorten URLs

* revert: changes to schema.rb

* modify: test to aggregate failures

Co-authored-by: rhymes <github@rhymes.dev>

Co-authored-by: rhymes <github@rhymes.dev>
---
 app/models/comment.rb       |  9 +++++++--
 spec/models/comment_spec.rb | 15 ++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/app/models/comment.rb b/app/models/comment.rb
index c3c460815..def361547 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -12,6 +12,8 @@ class Comment < ApplicationRecord
   TITLE_DELETED = "[deleted]".freeze
   TITLE_HIDDEN = "[hidden by post author]".freeze
 
+  URI_REGEXP = %r{(?<scheme>https?://)?(?<host>.+?)(?<port>:\d+)?$}.freeze
+
   # The date that we began limiting the number of user mentions in a comment.
   MAX_USER_MENTION_LIVE_AT = Time.utc(2021, 3, 12).freeze
 
@@ -202,9 +204,12 @@ class Comment < ApplicationRecord
   def shorten_urls!
     doc = Nokogiri::HTML.fragment(processed_html)
     doc.css("a").each do |anchor|
-      unless anchor.to_s.include?("<img") || anchor.to_s.include?("<del") || anchor.attr("class")&.include?("ltag")
-        anchor.content = strip_url(anchor.content) unless anchor.to_s.include?("<img") # rubocop:disable Style/SoleNestedConditional
+      anchor_inner_html = anchor.inner_html
+      urls = anchor_inner_html.scan(URI_REGEXP).flatten.compact
+      urls.each do |url|
+        anchor_inner_html.sub!(/#{Regexp.escape(url)}/, strip_url(url))
       end
+      anchor.inner_html = anchor_inner_html
     end
     self.processed_html = doc.to_html.html_safe # rubocop:disable Rails/OutputSafety
   end
diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb
index 84508991f..77805483f 100644
--- a/spec/models/comment_spec.rb
+++ b/spec/models/comment_spec.rb
@@ -184,11 +184,20 @@ RSpec.describe Comment, type: :model do
         expect(comment.processed_html.include?("Hello <a")).to be(true)
       end
 
-      it "shortens long urls" do
-        comment.body_markdown = "Hello https://longurl.com/#{'x' * 100}?#{'y' * 100}"
+      it "shortens long urls without removing formatting", :aggregate_failures do
+        long_url = "https://longurl.com/#{'x' * 100}?#{'y' * 100}"
+        comment.body_markdown = "Hello #{long_url}"
         comment.validate!
-        expect(comment.processed_html.include?("...</a>")).to be(true)
+        expect(comment.processed_html.include?("...")).to be(true)
         expect(comment.processed_html.size < 450).to be(true)
+
+        comment.body_markdown = "Hello [**#{long_url}**](#{long_url})"
+        comment.validate!
+        expect(comment.processed_html.include?("...</strong>")).to be(true)
+
+        comment.body_markdown = "Hello ![Alt-text](#{long_url})"
+        comment.validate!
+        expect(comment.processed_html.include?("<img")).to be(true)
       end
 
       # rubocop:disable RSpec/ExampleLength