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.
This commit is contained in:
Bianca Nenciu 2021-11-17 07:39:49 +02:00 committed by GitHub
parent 2ff7f105d9
commit cc1b45f58b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 14 deletions

View File

@ -161,12 +161,8 @@ class TopicEmbed < ActiveRecord::Base
src = node[url_param] src = node[url_param]
unless (src.nil? || src.empty?) unless (src.nil? || src.empty?)
begin begin
uri = URI.parse(UrlHelper.escape_uri(src)) # convert URL to absolute form
unless uri.host node[url_param] = URI.join(url, UrlHelper.escape_uri(src)).to_s
uri.scheme = original_uri.scheme
uri.host = original_uri.host
node[url_param] = uri.to_s
end
rescue URI::Error, Addressable::URI::InvalidURIError rescue URI::Error, Addressable::URI::InvalidURIError
# If there is a mistyped URL, just do nothing # If there is a mistyped URL, just do nothing
end end
@ -211,15 +207,13 @@ class TopicEmbed < ActiveRecord::Base
fragment = Nokogiri::HTML5.fragment("<div>#{contents}</div>") fragment = Nokogiri::HTML5.fragment("<div>#{contents}</div>")
fragment.css('a').each do |a| fragment.css('a').each do |a|
href = a['href'] if a['href'].present?
if href.present? && href.start_with?('/') a['href'] = URI.join(prefix, a['href']).to_s
a['href'] = "#{prefix}/#{href.sub(/^\/+/, '')}"
end end
end end
fragment.css('img').each do |a| fragment.css('img').each do |a|
src = a['src'] if a['src'].present?
if src.present? && src.start_with?('/') a['src'] = URI.join(prefix, a['src']).to_s
a['src'] = "#{prefix}/#{src.sub(/^\/+/, '')}"
end end
end end
fragment.at('div').inner_html fragment.at('div').inner_html

View File

@ -14,7 +14,7 @@ describe TopicEmbed do
fab!(:user) { Fabricate(:user) } fab!(:user) { Fabricate(:user) }
let(:title) { "How to turn a fish from good to evil in 30 seconds" } let(:title) { "How to turn a fish from good to evil in 30 seconds" }
let(:url) { 'http://eviltrout.com/123' } 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!(:embeddable_host) { Fabricate(:embeddable_host) }
fab!(:category) { Fabricate(:category) } fab!(:category) { Fabricate(:category) }
fab!(:tag) { Fabricate(:tag) } 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('a', with: { href: 'http://eviltrout.com/hello' })
expect(post.cooked).to have_tag('img', with: { src: 'http://eviltrout.com/images/wat.jpg' }) 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(post.topic.has_topic_embed?).to eq(true)
expect(TopicEmbed.where(topic_id: post.topic_id)).to be_present expect(TopicEmbed.where(topic_id: post.topic_id)).to be_present
@ -335,7 +339,7 @@ describe TopicEmbed do
it "handles mailto links" do it "handles mailto links" do
response = TopicEmbed.find_remote(url) 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' }) expect(response.body).to have_tag('a', with: { href: 'mailto:bar@example.com' })
end end
end end