Commit d2aa46b7 authored by Sean McGivern's avatar Sean McGivern Committed by Rémy Coutable

Merge branch '22782-external-link-filter-with-non-lowercase-scheme' into 'master'

Add Nofollow for uppercased external url protocols

Closes #22782

See merge request !6820
parent c2f0ea7d
...@@ -119,6 +119,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -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 - Retouch environments list and deployments list
- Add multiple command support for all label related slash commands !6780 (barthc) - 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 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) - Allow empty merge requests !6384 (Artem Sidorenko)
- Grouped pipeline dropdown is a scrollable container - Grouped pipeline dropdown is a scrollable container
- Cleanup Ci::ApplicationController. !6757 (Takuya Noguchi) - Cleanup Ci::ApplicationController. !6757 (Takuya Noguchi)
......
...@@ -3,10 +3,17 @@ module Banzai ...@@ -3,10 +3,17 @@ module Banzai
# HTML Filter to modify the attributes of external links # HTML Filter to modify the attributes of external links
class ExternalLinkFilter < HTML::Pipeline::Filter class ExternalLinkFilter < HTML::Pipeline::Filter
def call def call
# Skip non-HTTP(S) links and internal links links.each do |node|
doc.xpath("descendant-or-self::a[starts-with(@href, 'http') and not(starts-with(@href, '#{internal_url}'))]").each do |node| href = href_to_lowercase_scheme(node["href"].to_s)
node.set_attribute('rel', 'nofollow noreferrer')
node.set_attribute('target', '_blank') 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 end
doc doc
...@@ -14,6 +21,25 @@ module Banzai ...@@ -14,6 +21,25 @@ module Banzai
private 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 def internal_url
@internal_url ||= Gitlab.config.gitlab.url @internal_url ||= Gitlab.config.gitlab.url
end end
......
...@@ -46,4 +46,38 @@ describe Banzai::Filter::ExternalLinkFilter, lib: true do ...@@ -46,4 +46,38 @@ describe Banzai::Filter::ExternalLinkFilter, lib: true do
expect(doc.at_css('a')['rel']).to include 'noreferrer' expect(doc.at_css('a')['rel']).to include 'noreferrer'
end end
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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment