diff --git a/CHANGELOG.md b/CHANGELOG.md index c09957ba8ed88eb9b2aac6777f10e74f99673d03..a816e30a9c7d8d49f2446055bf46f8ceeaff117d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Retouch environments list and deployments list - Add multiple command support for all label related slash commands !6780 (barthc) - Add Container Registry on/off status to Admin Area !6638 (the-undefined) + - Add Nofollow for uppercased scheme in external urls !6820 (the-undefined) - Allow empty merge requests !6384 (Artem Sidorenko) - Grouped pipeline dropdown is a scrollable container - Cleanup Ci::ApplicationController. !6757 (Takuya Noguchi) diff --git a/lib/banzai/filter/external_link_filter.rb b/lib/banzai/filter/external_link_filter.rb index 0a29c547a4de70ec177eed8fc1112f0882654e4a..2f19b59e7252de0701adcfec839f412eb9415d27 100644 --- a/lib/banzai/filter/external_link_filter.rb +++ b/lib/banzai/filter/external_link_filter.rb @@ -3,10 +3,17 @@ module Banzai # HTML Filter to modify the attributes of external links class ExternalLinkFilter < HTML::Pipeline::Filter def call - # Skip non-HTTP(S) links and internal links - doc.xpath("descendant-or-self::a[starts-with(@href, 'http') and not(starts-with(@href, '#{internal_url}'))]").each do |node| - node.set_attribute('rel', 'nofollow noreferrer') - node.set_attribute('target', '_blank') + links.each do |node| + href = href_to_lowercase_scheme(node["href"].to_s) + + unless node["href"].to_s == href + node.set_attribute('href', href) + end + + if href =~ /\Ahttp(s)?:\/\// && external_url?(href) + node.set_attribute('rel', 'nofollow noreferrer') + node.set_attribute('target', '_blank') + end end doc @@ -14,6 +21,25 @@ module Banzai private + def links + query = 'descendant-or-self::a[@href and not(@href = "")]' + doc.xpath(query) + end + + def href_to_lowercase_scheme(href) + scheme_match = href.match(/\A(\w+):\/\//) + + if scheme_match + scheme_match.to_s.downcase + scheme_match.post_match + else + href + end + end + + def external_url?(url) + !url.start_with?(internal_url) + end + def internal_url @internal_url ||= Gitlab.config.gitlab.url end diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index 695a5bc6fd4418fe6d40575e4217e670f4de86ab..167397c736bdd8e6b15355411ab3057b926f083b 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -46,4 +46,38 @@ describe Banzai::Filter::ExternalLinkFilter, lib: true do expect(doc.at_css('a')['rel']).to include 'noreferrer' end end + + context 'for non-lowercase scheme links' do + let(:doc_with_http) { filter %q(<p><a href="httP://google.com/">Google</a></p>) } + let(:doc_with_https) { filter %q(<p><a href="hTTpS://google.com/">Google</a></p>) } + + it 'adds rel="nofollow" to external links' do + expect(doc_with_http.at_css('a')).to have_attribute('rel') + expect(doc_with_https.at_css('a')).to have_attribute('rel') + + expect(doc_with_http.at_css('a')['rel']).to include 'nofollow' + expect(doc_with_https.at_css('a')['rel']).to include 'nofollow' + end + + it 'adds rel="noreferrer" to external links' do + expect(doc_with_http.at_css('a')).to have_attribute('rel') + expect(doc_with_https.at_css('a')).to have_attribute('rel') + + expect(doc_with_http.at_css('a')['rel']).to include 'noreferrer' + expect(doc_with_https.at_css('a')['rel']).to include 'noreferrer' + end + + it 'skips internal links' do + internal_link = Gitlab.config.gitlab.url + "/sign_in" + url = internal_link.gsub(/\Ahttp/, 'HtTp') + act = %Q(<a href="#{url}">Login</a>) + exp = %Q(<a href="#{internal_link}">Login</a>) + expect(filter(act).to_html).to eq(exp) + end + + it 'skips relative links' do + exp = act = %q(<a href="http_spec/foo.rb">Relative URL</a>) + expect(filter(act).to_html).to eq(exp) + end + end end