diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index f255161f87a..6f11b3891c2 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -656,9 +656,8 @@ class CookedPostProcessor end def enforce_nofollow - if !@omit_nofollow && SiteSetting.add_rel_nofollow_to_user_content - PrettyText.add_rel_nofollow_to_user_content(@doc) - end + add_nofollow = !@omit_nofollow && SiteSetting.add_rel_nofollow_to_user_content + PrettyText.add_rel_attributes_to_user_content(@doc, add_nofollow) end def pull_hotlinked_images diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 743b28d351c..ec0936e6e43 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -269,9 +269,8 @@ module PrettyText doc = Nokogiri::HTML5.fragment(sanitized) - if !options[:omit_nofollow] && SiteSetting.add_rel_nofollow_to_user_content - add_rel_nofollow_to_user_content(doc) - end + add_nofollow = !options[:omit_nofollow] && SiteSetting.add_rel_nofollow_to_user_content + add_rel_attributes_to_user_content(doc, add_nofollow) if SiteSetting.enable_mentions add_mentions(doc, user_id: opts[:user_id]) @@ -284,7 +283,7 @@ module PrettyText loofah_fragment.scrub!(scrubber).to_html end - def self.add_rel_nofollow_to_user_content(doc) + def self.add_rel_attributes_to_user_content(doc, add_nofollow) allowlist = [] domains = SiteSetting.exclude_rel_nofollow_domains @@ -293,22 +292,21 @@ module PrettyText site_uri = nil doc.css("a").each do |l| href = l["href"].to_s + l["rel"] = "noopener" if l["target"] == "_blank" + begin uri = URI(UrlHelper.encode_component(href)) site_uri ||= URI(Discourse.base_url) - if !uri.host.present? || - uri.host == site_uri.host || - uri.host.ends_with?(".#{site_uri.host}") || - allowlist.any? { |u| uri.host == u || uri.host.ends_with?(".#{u}") } - # we are good no need for nofollow - l.remove_attribute("rel") - else - l["rel"] = "nofollow noopener" - end + same_domain = !uri.host.present? || + uri.host == site_uri.host || + uri.host.ends_with?(".#{site_uri.host}") || + allowlist.any? { |u| uri.host == u || uri.host.ends_with?(".#{u}") } + + l["rel"] = "noopener nofollow ugc" if add_nofollow && !same_domain rescue URI::Error # add a nofollow anyway - l["rel"] = "nofollow noopener" + l["rel"] = "noopener nofollow ugc" end end end diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 424ec994541..143536b7f50 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -237,13 +237,13 @@ describe CookedPostProcessor do count: 1 ) - expect(html).to have_tag("a[rel='nofollow noopener']") + expect(html).to have_tag("a[rel='noopener nofollow ugc']") end it 'removes nofollow if user is staff/tl3' do cpp = CookedPostProcessor.new(staff_post, invalidate_oneboxes: true) cpp.post_process - expect(cpp.html).to_not have_tag("a[rel='nofollow noopener']") + expect(cpp.html).to_not have_tag("a[rel='noopener nofollow ugc']") end end end @@ -1102,7 +1102,7 @@ describe CookedPostProcessor do SiteSetting.add_rel_nofollow_to_user_content = false Oneboxer.expects(:onebox) .with("http://www.youtube.com/watch?v=9bZkp7q19f0", invalidate_oneboxes: true, user_id: nil, category_id: post.topic.category_id) - .returns('') + .returns('') cpp.post_process_oneboxes end @@ -1123,7 +1123,7 @@ describe CookedPostProcessor do SiteSetting.tl3_links_no_follow = false Oneboxer.expects(:onebox) .with("http://www.youtube.com/watch?v=9bZkp7q19f0", invalidate_oneboxes: true, user_id: nil, category_id: post.topic.category_id) - .returns('') + .returns('') cpp.post_process_oneboxes end @@ -1204,7 +1204,7 @@ describe CookedPostProcessor do expect(cpp.html).to match_html <<~HTML.rstrip

Link

- Google
+ Google

text.txt (20 Bytes)
:smile:

@@ -1219,7 +1219,7 @@ describe CookedPostProcessor do expect(cpp.html).to match_html <<~HTML.rstrip

Link

- Google
+ Google

text.txt (20 Bytes)
:smile:

@@ -1232,7 +1232,7 @@ describe CookedPostProcessor do expect(cpp.html).to match_html <<~HTML.rstrip

Link

- Google
+ Google

text.txt (20 Bytes)
:smile:

@@ -1246,7 +1246,7 @@ describe CookedPostProcessor do expect(cpp.html).to match_html <<~HTML.rstrip

Link

- Google
+ Google

text.txt (20 Bytes)
:smile:

@@ -1260,7 +1260,7 @@ describe CookedPostProcessor do expect(cpp.html).to match_html <<~HTML.rstrip

Link

- Google
+ Google

text.txt (20 Bytes)
:smile:

diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index 72e22124730..12106c4ba2d 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -425,7 +425,7 @@ describe PrettyText do end it "can handle mentions inside a hyperlink" do - expect(PrettyText.cook("[link @inner](http://site.com)")).to match_html '

link @inner

' + expect(PrettyText.cook("[link @inner](http://site.com)")).to match_html '

link @inner

' end it "can handle a list of mentions" do @@ -513,42 +513,56 @@ describe PrettyText do end end - describe "rel nofollow" do + describe "rel attributes" do before do SiteSetting.add_rel_nofollow_to_user_content = true SiteSetting.exclude_rel_nofollow_domains = "foo.com|bar.com" end it "should inject nofollow in all user provided links" do - expect(PrettyText.cook('cnn')).to match(/nofollow noopener/) + expect(PrettyText.cook('cnn')).to match(/noopener nofollow ugc/) end it "should not inject nofollow in all local links" do - expect(PrettyText.cook("cnn") !~ /nofollow/).to eq(true) + expect(PrettyText.cook("cnn") !~ /nofollow ugc/).to eq(true) end it "should not inject nofollow in all subdomain links" do - expect(PrettyText.cook("cnn") !~ /nofollow/).to eq(true) + expect(PrettyText.cook("cnn") !~ /nofollow ugc/).to eq(true) end it "should inject nofollow in all non subdomain links" do - expect(PrettyText.cook("cnn")).to match(/nofollow/) + expect(PrettyText.cook("cnn")).to match(/nofollow ugc/) end it "should not inject nofollow for foo.com" do - expect(PrettyText.cook("cnn") !~ /nofollow/).to eq(true) + expect(PrettyText.cook("cnn") !~ /nofollow ugc/).to eq(true) end it "should inject nofollow for afoo.com" do - expect(PrettyText.cook("cnn")).to match(/nofollow/) + expect(PrettyText.cook("cnn")).to match(/nofollow ugc/) end it "should not inject nofollow for bar.foo.com" do - expect(PrettyText.cook("cnn") !~ /nofollow/).to eq(true) + expect(PrettyText.cook("cnn") !~ /nofollow ugc/).to eq(true) end it "should not inject nofollow if omit_nofollow option is given" do - expect(PrettyText.cook('cnn', omit_nofollow: true) !~ /nofollow/).to eq(true) + expect(PrettyText.cook('cnn', omit_nofollow: true) !~ /nofollow ugc/).to eq(true) + end + + it 'adds the noopener attribute even if omit_nofollow option is given' do + raw_html = 'Check out my site!' + expect( + PrettyText.cook(raw_html, omit_nofollow: true) + ).to match(/noopener/) + end + + it 'adds the noopener attribute even if omit_nofollow option is given' do + raw_html = 'Check out my site!' + expect( + PrettyText.cook(raw_html, omit_nofollow: false) + ).to match(/noopener nofollow ugc/) end end @@ -724,7 +738,7 @@ describe PrettyText do more stuff RAW post = Fabricate(:post, raw: raw) - expect(post.excerpt).to eq("hello site") + expect(post.excerpt).to eq("hello site") end it "handles span excerpt at the beginning of a post" do @@ -1109,7 +1123,7 @@ describe PrettyText do it "supports href schemes" do SiteSetting.allowed_href_schemes = "macappstore|steam" cooked = cook("[Steam URL Scheme](steam://store/452530)") - expected = '

Steam URL Scheme

' + expected = '

Steam URL Scheme

' expect(cooked).to eq(n expected) end @@ -1123,7 +1137,7 @@ describe PrettyText do it 'allows only tel URL scheme to start with a plus character' do SiteSetting.allowed_href_schemes = "tel|steam" cooked = cook("[Tel URL Scheme](tel://+452530579785)") - expected = '

Tel URL Scheme

' + expected = '

Tel URL Scheme

' expect(cooked).to eq(n expected) cooked2 = cook("[Steam URL Scheme](steam://+store/452530)") @@ -1151,7 +1165,7 @@ describe PrettyText do cooked = PrettyText.cook("[`a` #known::tag here](http://example.com)") html = <<~HTML -

a #known::tag here

+

a #known::tag here

HTML expect(cooked).to eq(html.strip) @@ -1251,7 +1265,7 @@ HTML it "won't break links by censoring them." do expect_cooked_match("The link still works. [whiz](http://www.whiz.com)", - '

The link still works. ■■■■

') + '

The link still works. ■■■■

') end it "escapes regexp characters" do @@ -1415,19 +1429,19 @@ HTML it "supports url bbcode" do cooked = PrettyText.cook "[url]http://sam.com[/url]" - html = '

http://sam.com

' + html = '

http://sam.com

' expect(cooked).to eq(html) end it "supports nesting tags in url" do cooked = PrettyText.cook("[url=http://sam.com][b]I am sam[/b][/url]") - html = '

I am sam

' + html = '

I am sam

' expect(cooked).to eq(html) end it "supports query params in bbcode url" do cooked = PrettyText.cook("[url=https://www.amazon.com/Camcorder-Hausbell-302S-Control-Infrared/dp/B01KLOA1PI/?tag=discourse]BBcode link[/url]") - html = '

BBcode link

' + html = '

BBcode link

' expect(cooked).to eq(html) end @@ -1445,13 +1459,13 @@ HTML it "support special handling for space in urls" do cooked = PrettyText.cook "http://testing.com?a%20b" - html = '

http://testing.com?a%20b

' + html = '

http://testing.com?a%20b

' expect(cooked).to eq(html) end it "supports onebox for decoded urls" do cooked = PrettyText.cook "http://testing.com?a%50b" - html = '

http://testing.com?aPb

' + html = '

http://testing.com?aPb

' expect(cooked).to eq(html) end @@ -1613,7 +1627,7 @@ HTML cooked = PrettyText.cook(md) html = <<~HTML -

www.cnn.com test.it http://test.com https://test.ab https://a

+

www.cnn.com test.it http://test.com https://test.ab https://a

HTML expect(cooked).to eq(html.strip) @@ -1623,7 +1637,7 @@ HTML cooked = PrettyText.cook(md) html = <<~HTML -

www.cnn.com test.it http://test.com https://test.ab https://a

+

www.cnn.com test.it http://test.com https://test.ab https://a

HTML expect(cooked).to eq(html.strip) @@ -1633,7 +1647,7 @@ HTML cooked = PrettyText.cook(md) html = <<~HTML -

www.cnn.com test.it http://test.com https://test.ab https://a

+

www.cnn.com test.it http://test.com https://test.ab https://a

HTML expect(cooked).to eq(html.strip) diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 5acc196a7ff..6819e8b3172 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -1025,7 +1025,7 @@ describe Post do it "should add nofollow to links in the post for trust levels below 3" do post.user.trust_level = 2 post.save - expect(post.cooked).to match(/nofollow noopener/) + expect(post.cooked).to match(/noopener nofollow ugc/) end it "when tl3_links_no_follow is false, should not add nofollow for trust level 3 and higher" do @@ -1039,7 +1039,7 @@ describe Post do SiteSetting.tl3_links_no_follow = true post.user.trust_level = 3 post.save - expect(post.cooked).to match(/nofollow noopener/) + expect(post.cooked).to match(/noopener nofollow ugc/) end describe 'mentions' do diff --git a/spec/models/user_profile_spec.rb b/spec/models/user_profile_spec.rb index 65221592b0b..36e21ff5504 100644 --- a/spec/models/user_profile_spec.rb +++ b/spec/models/user_profile_spec.rb @@ -138,8 +138,8 @@ describe UserProfile do it 'includes the link as nofollow if the user is not new' do user.user_profile.send(:cook) - expect(user_profile.bio_excerpt).to match_html("I love http://discourse.org") - expect(user_profile.bio_processed).to match_html("

I love http://discourse.org

") + expect(user_profile.bio_excerpt).to match_html("I love http://discourse.org") + expect(user_profile.bio_processed).to match_html("

I love http://discourse.org

") end it 'removes the link if the user is new' do @@ -177,8 +177,8 @@ describe UserProfile do created_user.save created_user.reload created_user.change_trust_level!(TrustLevel[2]) - expect(created_user.user_profile.bio_excerpt).to match_html("I love http://discourse.org") - expect(created_user.user_profile.bio_processed).to match_html("

I love http://discourse.org

") + expect(created_user.user_profile.bio_excerpt).to match_html("I love http://discourse.org") + expect(created_user.user_profile.bio_processed).to match_html("

I love http://discourse.org

") end end @@ -188,8 +188,8 @@ describe UserProfile do it 'includes the link with nofollow if the user is trust level 3 or higher' do user.trust_level = TrustLevel[3] user_profile.send(:cook) - expect(user_profile.bio_excerpt).to match_html("I love http://discourse.org") - expect(user_profile.bio_processed).to match_html("

I love http://discourse.org

") + expect(user_profile.bio_excerpt).to match_html("I love http://discourse.org") + expect(user_profile.bio_processed).to match_html("

I love http://discourse.org

") end end end