Render references for labels that name contains ?, or &

6 files changed, 77 insertions, 17 deletions

diff --git a/app/assets/javascripts/gfm_auto_complete.js.coffee b/app/assets/javascripts/gfm_auto_complete.js.coffee
index b7d040bae85..4a851d9c9fb 100644
--- a/app/assets/javascripts/gfm_auto_complete.js.coffee
+++ b/app/assets/javascripts/gfm_auto_complete.js.coffee
@@ -190,7 +190,7 @@ GitLab.GfmAutoComplete =
       callbacks:
         beforeSave: (merges) ->
           sanitizeLabelTitle = (title)->
-            if /\w+\s+\w+/g.test(title)
+            if /[\w\?&]+\s+[\w\?&]+/g.test(title)
               "\"#{sanitize(title)}\""
             else
               sanitize(title)
diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb
index 5e9f5837101..1f0d5d545c0 100644
--- a/app/helpers/labels_helper.rb
+++ b/app/helpers/labels_helper.rb
@@ -1,6 +1,12 @@
 module LabelsHelper
   include ActionView::Helpers::TagHelper
 
+  TABLE_FOR_ESCAPE_HTML_ENTITIES = {
+    '&' => '&',
+    '<' => '&lt;',
+    '>' => '&gt;'
+  }
+
   # Link to a Label
   #
   # label   - Label object to link to
@@ -130,7 +136,11 @@ module LabelsHelper
     label.subscribed?(current_user) ? 'Unsubscribe' : 'Subscribe'
   end
 
+  def unescape_html_entities(value)
+    value.to_s.gsub(/(&gt;)|(&lt;)|(&amp;)/, TABLE_FOR_ESCAPE_HTML_ENTITIES.invert)
+  end
+
   # Required for Banzai::Filter::LabelReferenceFilter
   module_function :render_colored_label, :render_colored_cross_project_label,
-                  :text_color_for_bg, :escape_once
+                  :text_color_for_bg, :escape_once, :unescape_html_entities
 end
diff --git a/app/models/label.rb b/app/models/label.rb
index 115f38c6dfe..086007d1864 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -58,8 +58,8 @@ class Label < ActiveRecord::Base
       (?:
         (?<label_id>\d+) | # Integer-based label ID, or
         (?<label_name>
-          [A-Za-z0-9_-]+ | # String-based single-word label title, or
-          "[^&\?,]+"       # String-based multi-word label surrounded in quotes
+          [A-Za-z0-9_\-\?&]+ | # String-based single-word label title, or
+          "[^,]+"              # String-based multi-word label surrounded in quotes
         )
       )
     }x
@@ -134,16 +134,6 @@ class Label < ActiveRecord::Base
   end
 
   def sanitize_title(value)
-    unnescape_html_entities(Sanitize.clean(value.to_s))
+    LabelsHelper.unescape_html_entities(Sanitize.clean(value.to_s))
   end
-
-  def unnescape_html_entities(value)
-    value.to_s.gsub(/(&gt;)|(&lt;)|(&amp;)/, Label::TABLE_FOR_ESCAPE_HTML_ENTITIES.invert)
-  end
-
-  TABLE_FOR_ESCAPE_HTML_ENTITIES = {
-    '&' => '&amp;',
-    '<' => '&lt;',
-    '>' => '&gt;'
-  }
 end
diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb
index e4d3f87d0aa..7d016d78669 100644
--- a/lib/banzai/filter/label_reference_filter.rb
+++ b/lib/banzai/filter/label_reference_filter.rb
@@ -13,13 +13,13 @@ module Banzai
       end
 
       def self.references_in(text, pattern = Label.reference_pattern)
-        text.gsub(pattern) do |match|
+        unescape_html_entities(text).gsub(pattern) do |match|
           yield match, $~[:label_id].to_i, $~[:label_name], $~[:project], $~
         end
       end
 
       def references_in(text, pattern = Label.reference_pattern)
-        text.gsub(pattern) do |match|
+        unescape_html_entities(text).gsub(pattern) do |match|
           label = find_label($~[:project], $~[:label_id], $~[:label_name])
 
           if label
@@ -66,6 +66,10 @@ module Banzai
           LabelsHelper.render_colored_cross_project_label(object)
         end
       end
+
+      def unescape_html_entities(text)
+        LabelsHelper.unescape_html_entities(text)
+      end
     end
   end
 end
diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb
index 501f150cfda..1457eea7cb2 100644
--- a/spec/helpers/labels_helper_spec.rb
+++ b/spec/helpers/labels_helper_spec.rb
@@ -77,4 +77,10 @@ describe LabelsHelper do
       expect(text_color_for_bg('#000')).to eq '#FFFFFF'
     end
   end
+
+  describe 'unescape_html_entities' do
+    it 'decodes &, <, and > named entities' do
+      expect(unescape_html_entities('foo &amp; bar &lt; zoo &gt; boo &eacute;')).to eq 'foo & bar < zoo > boo &eacute;'
+    end
+  end
 end
diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb
index f1064a701d8..9e3d2f5825d 100644
--- a/spec/lib/banzai/filter/label_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb
@@ -104,6 +104,31 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
     end
   end
 
+  context 'String-based single-word references with special characters' do
+    let(:label)     { create(:label, name: '?gfm&', project: project) }
+    let(:reference) { "#{Label.reference_prefix}#{label.name}" }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_issues_url(project.namespace, project, label_name: label.name)
+      expect(doc.text).to eq 'See ?gfm&'
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Label (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+><span.+>\?gfm&amp;</span></a>\.\)))
+    end
+
+    it 'ignores invalid label names' do
+      act = "Label #{Label.reference_prefix}#{label.name.reverse}"
+      exp = "Label #{Label.reference_prefix}&amp;mfg?"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
   context 'String-based multi-word references in quotes' do
     let(:label)     { create(:label, name: 'gfm references', project: project) }
     let(:reference) { label.to_reference(format: :name) }
@@ -128,6 +153,31 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
     end
   end
 
+  context 'String-based multi-word references with special characters in quotes' do
+    let(:label)     { create(:label, name: 'gfm & references?', project: project) }
+    let(:reference) { label.to_reference(format: :name) }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_issues_url(project.namespace, project, label_name: label.name)
+      expect(doc.text).to eq 'See gfm & references?'
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Label (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+><span.+>gfm &amp; references\?</span></a>\.\)))
+    end
+
+    it 'ignores invalid label names' do
+      act = %(Label #{Label.reference_prefix}"#{label.name.reverse}")
+      exp = %(Label #{Label.reference_prefix}"?secnerefer &amp; mfg\")
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
   describe 'edge cases' do
     it 'gracefully handles non-references matching the pattern' do
       exp = act = '(format nil "~0f" 3.0) ; 3.0'