Merge branch '18591-banzai-filter-upload-link-filter' into 'master'

Banzai::Filter::UploadLinkFilter use XPath

See merge request !4703

3 files changed, 22 insertions, 10 deletions

diff --git a/CHANGELOG b/CHANGELOG
index fa0960b2847..39532e88138 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -118,6 +118,7 @@ v 8.8.5
   - Prevent unauthorized access for projects build traces
   - 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
 
 v 8.8.4
   - Fix LDAP-based login for users with 2FA enabled. !4493
diff --git a/lib/banzai/filter/upload_link_filter.rb b/lib/banzai/filter/upload_link_filter.rb
index c0f503c9af3..45bb66dc99f 100644
--- a/lib/banzai/filter/upload_link_filter.rb
+++ b/lib/banzai/filter/upload_link_filter.rb
@@ -10,11 +10,11 @@ module Banzai
       def call
         return doc unless project
 
-        doc.search('a').each do |el|
+        doc.xpath('descendant-or-self::a[starts-with(@href, "/uploads/")]').each do |el|
           process_link_attr el.attribute('href')
         end
 
-        doc.search('img').each do |el|
+        doc.xpath('descendant-or-self::img[starts-with(@src, "/uploads/")]').each do |el|
           process_link_attr el.attribute('src')
         end
 
@@ -24,12 +24,7 @@ module Banzai
       protected
 
       def process_link_attr(html_attr)
-        return if html_attr.blank?
-
-        uri = html_attr.value
-        if uri.starts_with?("/uploads/")
-          html_attr.value = build_url(uri).to_s
-        end
+        html_attr.value = build_url(html_attr.value).to_s
       end
 
       def build_url(uri)
diff --git a/spec/lib/banzai/filter/upload_link_filter_spec.rb b/spec/lib/banzai/filter/upload_link_filter_spec.rb
index b83be54746c..273d2ed709a 100644
--- a/spec/lib/banzai/filter/upload_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/upload_link_filter_spec.rb
@@ -23,6 +23,14 @@ describe Banzai::Filter::UploadLinkFilter, lib: true do
     %(<a href="#{path}">#{path}</a>)
   end
 
+  def nested_image(path)
+    %(<div><img src="#{path}" /></div>)
+  end
+
+  def nested_link(path)
+    %(<div><a href="#{path}">#{path}</a></div>)
+  end
+
   let(:project) { create(:project) }
 
   shared_examples :preserve_unchanged do
@@ -47,11 +55,19 @@ describe Banzai::Filter::UploadLinkFilter, lib: true do
       doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
       expect(doc.at_css('a')['href']).
         to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
+
+      doc = filter(nested_link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
+      expect(doc.at_css('a')['href']).
+        to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
     end
 
     it 'rebuilds relative URL for an image' do
-      doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
-      expect(doc.at_css('a')['href']).
+      doc = filter(image('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
+      expect(doc.at_css('img')['src']).
+        to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
+
+      doc = filter(nested_image('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
+      expect(doc.at_css('img')['src']).
         to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
     end