Merge branch 'gh-labels' into 'master'

Allow `?`, or `&` for label names

Closes #18727 

See merge request !4724

4 files changed, 16 insertions, 13 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/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee
index ce859fedb2d..7688609b301 100644
--- a/app/assets/javascripts/labels_select.js.coffee
+++ b/app/assets/javascripts/labels_select.js.coffee
@@ -32,7 +32,7 @@ class @LabelsSelect
       if issueUpdateURL
         labelHTMLTemplate = _.template(
             '<% _.each(labels, function(label){ %>
-            <a href="<%- ["",issueURLSplit[1], issueURLSplit[2],""].join("/") %>issues?label_name[]=<%- label.title %>">
+            <a href="<%- ["",issueURLSplit[1], issueURLSplit[2],""].join("/") %>issues?label_name[]=<%- encodeURIComponent(label.title) %>">
             <span class="label has-tooltip color-label" title="<%- label.description %>" style="background-color: <%- label.color %>; color: <%- label.text_color %>;">
             <%- label.title %>
             </span>
@@ -261,7 +261,7 @@ class @LabelsSelect
             $a.attr('data-label-id', label.id)
 
           $a.addClass(selectedClass.join(' '))
-            .html("#{colorEl} #{_.escape(label.title)}")
+            .html("#{colorEl} #{label.title}")
 
           # Return generated html
           $li.html($a).prop('outerHTML')
@@ -288,7 +288,7 @@ class @LabelsSelect
         fieldName: $dropdown.data('field-name')
         id: (label) ->
           if $dropdown.hasClass("js-filter-submit") and not label.isAny?
-            _.escape label.title
+            label.title
           else
             label.id
 
diff --git a/app/models/label.rb b/app/models/label.rb
index 49c352cc239..dc5586f5756 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -20,10 +20,10 @@ class Label < ActiveRecord::Base
   validates :color, color: true, allow_blank: false
   validates :project, presence: true, unless: Proc.new { |service| service.template? }
 
-  # Don't allow '?', '&', and ',' for label titles
+  # Don't allow ',' for label titles
   validates :title,
             presence: true,
-            format: { with: /\A[^&\?,]+\z/ },
+            format: { with: /\A[^,]+\z/ },
             uniqueness: { scope: :project_id }
 
   before_save :nullify_priority
@@ -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
@@ -114,7 +114,7 @@ class Label < ActiveRecord::Base
   end
 
   def title=(value)
-    write_attribute(:title, Sanitize.clean(value.to_s)) if value.present?
+    write_attribute(:title, sanitize_title(value)) if value.present?
   end
 
   private
@@ -132,4 +132,8 @@ class Label < ActiveRecord::Base
   def nullify_priority
     self.priority = nil if priority.blank?
   end
+
+  def sanitize_title(value)
+    CGI.unescapeHTML(Sanitize.clean(value.to_s))
+  end
 end
diff --git a/app/views/shared/_labels_row.html.haml b/app/views/shared/_labels_row.html.haml
index 5507a05f6c1..dce492352ac 100644
--- a/app/views/shared/_labels_row.html.haml
+++ b/app/views/shared/_labels_row.html.haml
@@ -1,10 +1,9 @@
 - labels.each do |label|
-  %span.label-row.btn-group{ role: "group", aria: { label: escape_once(label.name) }, style: "color: #{text_color_for_bg(label.color)}" }
-    = link_to label_filter_path(@project, label, type: controller.controller_name),
+  %span.label-row.btn-group{ role: "group", aria: { label: label.name }, style: "color: #{text_color_for_bg(label.color)}" }
+    = link_to label.name, label_filter_path(@project, label, type: controller.controller_name),
       class: "btn btn-transparent has-tooltip",
       style: "background-color: #{label.color};",
       title: escape_once(label.description),
-      data: { container: "body" } do
-      = escape_once label.name
+      data: { container: "body" }
     %button.btn.btn-transparent.label-remove.js-label-filter-remove{ type: "button", style: "background-color: #{label.color};", data: { label: label.title } }
       = icon("times")