Fix milestone references with HTML entities in the name

When a milestone name contained an HTML entity that would be escaped (&, <, >), then it wasn't possible to refer to this milestone by name, or use it in a quick action. This already worked for labels, but not for milestones. We take care to re-escape un-matched milestones, too.

Fix milestone references with HTML entities in the name
17b97bf0 · Sean McGivern · efdca2e6 · 17b97bf0 · 17b97bf0 · 17b97bf0
Commit 17b97bf0 authored 5 years ago by Sean McGivern
--- a/changelogs/unreleased/fix-milestone-references-with-escaped-html-entities.yml
+++ b/changelogs/unreleased/fix-milestone-references-with-escaped-html-entities.yml
+---
+title: Fix milestone references containing &, <, or >
+merge_request: 28667
+author:
+type: fixed
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -363,6 +363,14 @@ module Banzai
  
        group_ref
      end
+
+      def unescape_html_entities(text)
+        CGI.unescapeHTML(text.to_s)
+      end
+
+      def escape_html_entities(text)
+        CGI.escapeHTML(text.to_s)
+      end
    end
  end
 end
--- a/lib/banzai/filter/label_reference_filter.rb
+++ b/lib/banzai/filter/label_reference_filter.rb
@@ -104,14 +104,6 @@ module Banzai
        matches[:namespace] && matches[:project]
      end
  
-      def unescape_html_entities(text)
-        CGI.unescapeHTML(text.to_s)
-      end
-
-      def escape_html_entities(text)
-        CGI.escapeHTML(text.to_s)
-      end
-
      def object_link_title(object, matches)
        # use title of wrapped element instead
        nil

--- a/lib/banzai/filter/milestone_reference_filter.rb
+++ b/lib/banzai/filter/milestone_reference_filter.rb
@@ -51,13 +51,13 @@ module Banzai
        # default implementation.
        return super(text, pattern) if pattern != Milestone.reference_pattern
  
-        text.gsub(pattern) do |match|
+        unescape_html_entities(text).gsub(pattern) do |match|
          milestone = find_milestone($~[:project], $~[:namespace], $~[:milestone_iid], $~[:milestone_name])
  
          if milestone
            yield match, milestone.id, $~[:project], $~[:namespace], $~
          else
-            match
+            escape_html_entities(match)
          end
        end
      end

--- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb
@@ -295,6 +295,25 @@ describe Banzai::Filter::MilestoneReferenceFilter do
    end
  end
  
+  shared_examples 'references with HTML entities' do
+    before do
+      milestone.update!(title: '&lt;html&gt;')
+    end
+
+    it 'links to a valid reference' do
+      doc = reference_filter('See %"&lt;html&gt;"')
+
+      expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
+      expect(doc.text).to eq 'See %<html>'
+    end
+
+    it 'ignores invalid milestone names and escapes entities' do
+      act = %(Milestone %"&lt;non valid&gt;")
+
+      expect(reference_filter(act).to_html).to eq act
+    end
+  end
+
  shared_context 'project milestones' do
    let(:reference) { milestone.to_reference(format: :iid) }
  
@@ -307,6 +326,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do
    it_behaves_like 'cross-project / cross-namespace complete reference'
    it_behaves_like 'cross-project / same-namespace complete reference'
    it_behaves_like 'cross project shorthand reference'
+    it_behaves_like 'references with HTML entities'
  end
  
  shared_context 'group milestones' do
@@ -317,6 +337,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do
    it_behaves_like 'String-based single-word references'
    it_behaves_like 'String-based multi-word references in quotes'
    it_behaves_like 'referencing a milestone in a link href'
+    it_behaves_like 'references with HTML entities'
  
    it 'does not support references by IID' do
      doc = reference_filter("See #{Milestone.reference_prefix}#{milestone.iid}")