From cc1b45f58b3ecb8eeeeea5b9169006f332db187e Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Wed, 17 Nov 2021 07:39:49 +0200
Subject: [PATCH] FIX: Convert URLs embedded topics to absolute form (#14975)

Sometimes the expanded post contained broken relative URLs because they
were not converted to their absolute form.
---
 app/models/topic_embed.rb       | 18 ++++++------------
 spec/models/topic_embed_spec.rb |  8 ++++++--
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb
index 22739134d1e..010676cb162 100644
--- a/app/models/topic_embed.rb
+++ b/app/models/topic_embed.rb
@@ -161,12 +161,8 @@ class TopicEmbed < ActiveRecord::Base
       src = node[url_param]
       unless (src.nil? || src.empty?)
         begin
-          uri = URI.parse(UrlHelper.escape_uri(src))
-          unless uri.host
-            uri.scheme = original_uri.scheme
-            uri.host = original_uri.host
-            node[url_param] = uri.to_s
-          end
+          # convert URL to absolute form
+          node[url_param] = URI.join(url, UrlHelper.escape_uri(src)).to_s
         rescue URI::Error, Addressable::URI::InvalidURIError
           # If there is a mistyped URL, just do nothing
         end
@@ -211,15 +207,13 @@ class TopicEmbed < ActiveRecord::Base
 
     fragment = Nokogiri::HTML5.fragment("<div>#{contents}</div>")
     fragment.css('a').each do |a|
-      href = a['href']
-      if href.present? && href.start_with?('/')
-        a['href'] = "#{prefix}/#{href.sub(/^\/+/, '')}"
+      if a['href'].present?
+        a['href'] = URI.join(prefix, a['href']).to_s
       end
     end
     fragment.css('img').each do |a|
-      src = a['src']
-      if src.present? && src.start_with?('/')
-        a['src'] = "#{prefix}/#{src.sub(/^\/+/, '')}"
+      if a['src'].present?
+        a['src'] = URI.join(prefix, a['src']).to_s
       end
     end
     fragment.at('div').inner_html
diff --git a/spec/models/topic_embed_spec.rb b/spec/models/topic_embed_spec.rb
index 4ce37dc37a1..8913e9b3d8e 100644
--- a/spec/models/topic_embed_spec.rb
+++ b/spec/models/topic_embed_spec.rb
@@ -14,7 +14,7 @@ describe TopicEmbed do
     fab!(:user) { Fabricate(:user) }
     let(:title) { "How to turn a fish from good to evil in 30 seconds" }
     let(:url) { 'http://eviltrout.com/123' }
-    let(:contents) { "<p>hello world new post <a href='/hello'>hello</a> <img src='/images/wat.jpg'></p>" }
+    let(:contents) { "<p>hello world new post <a href='/hello'>hello</a> <img src='images/wat.jpg'></p>" }
     fab!(:embeddable_host) { Fabricate(:embeddable_host) }
     fab!(:category) { Fabricate(:category) }
     fab!(:tag) { Fabricate(:tag) }
@@ -39,6 +39,10 @@ describe TopicEmbed do
         expect(post.cooked).to have_tag('a', with: { href: 'http://eviltrout.com/hello' })
         expect(post.cooked).to have_tag('img', with: { src: 'http://eviltrout.com/images/wat.jpg' })
 
+        # It converts relative URLs to absolute when expanded
+        stub_request(:get, url).to_return(status: 200, body: contents)
+        expect(TopicEmbed.expanded_for(post)).to have_tag('img', with: { src: 'http://eviltrout.com/images/wat.jpg' })
+
         expect(post.topic.has_topic_embed?).to eq(true)
         expect(TopicEmbed.where(topic_id: post.topic_id)).to be_present
 
@@ -335,7 +339,7 @@ describe TopicEmbed do
       it "handles mailto links" do
         response = TopicEmbed.find_remote(url)
 
-        expect(response.body).to have_tag('a', with: { href: 'mailto:foo%40example.com' })
+        expect(response.body).to have_tag('a', with: { href: 'mailto:foo@example.com' })
         expect(response.body).to have_tag('a', with: { href: 'mailto:bar@example.com' })
       end
     end