Banzai::Filter::ExternalLinkFilter use XPath

CHANGELOG

@@ -119,6 +119,7 @@ v 8.8.5
   - Forbid scripting for wiki files
   - Only show notes through JSON on confidential issues that the user has access to
   - Banzai::Filter::UploadLinkFilter use XPath instead CSS expressions
+  - Banzai::Filter::ExternalLinkFilter use XPath instead CSS expressions
 
 v 8.8.4
   - Fix LDAP-based login for users with 2FA enabled. !4493

lib/banzai/filter/external_link_filter.rb

@@ -3,17 +3,8 @@ module Banzai
     # HTML Filter to modify the attributes of external links
     class ExternalLinkFilter < HTML::Pipeline::Filter
       def call
-        doc.search('a').each do |node|
-          link = node.attr('href')
-
-          next unless link
-
-          # Skip non-HTTP(S) links
-          next unless link.start_with?('http')
-
-          # Skip internal links
-          next if link.start_with?(internal_url)
-
+        # 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')
         end

spec/lib/banzai/filter/external_link_filter_spec.rb

@@ -19,19 +19,31 @@ describe Banzai::Filter::ExternalLinkFilter, lib: true do
     expect(filter(act).to_html).to eq exp
   end
 
-  it 'adds rel="nofollow" to external links' do
-    act = %q(<a href="https://google.com/">Google</a>)
-    doc = filter(act)
-
-    expect(doc.at_css('a')).to have_attribute('rel')
-    expect(doc.at_css('a')['rel']).to include 'nofollow'
+  context 'for root links on document' do
+    let(:doc) { filter %q(<a href="https://google.com/">Google</a>) }
+
+    it 'adds rel="nofollow" to external links' do
+      expect(doc.at_css('a')).to have_attribute('rel')
+      expect(doc.at_css('a')['rel']).to include 'nofollow'
+    end
+
+    it 'adds rel="noreferrer" to external links' do
+      expect(doc.at_css('a')).to have_attribute('rel')
+      expect(doc.at_css('a')['rel']).to include 'noreferrer'
+    end
   end
 
-  it 'adds rel="noreferrer" to external links' do
-    act = %q(<a href="https://google.com/">Google</a>)
-    doc = filter(act)
+  context 'for nested links on document' do
+    let(:doc) { filter %q(<p><a href="https://google.com/">Google</a></p>) }
+
+    it 'adds rel="nofollow" to external links' do
+      expect(doc.at_css('a')).to have_attribute('rel')
+      expect(doc.at_css('a')['rel']).to include 'nofollow'
+    end
 
-    expect(doc.at_css('a')).to have_attribute('rel')
-    expect(doc.at_css('a')['rel']).to include 'noreferrer'
+    it 'adds rel="noreferrer" to external links' do
+      expect(doc.at_css('a')).to have_attribute('rel')
+      expect(doc.at_css('a')['rel']).to include 'noreferrer'
+    end
   end
 end