diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee
index ce859fedb2de2aa6ea5df34eae2b8bc73e237329..b88bc402801de8f7bfc003dd71abbdeecf440cbd 100644
--- a/app/assets/javascripts/labels_select.js.coffee
+++ b/app/assets/javascripts/labels_select.js.coffee
@@ -32,9 +32,9 @@ 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 %>
+ <%= label.title %>
</span>
</a>
<% }); %>'
diff --git a/app/models/label.rb b/app/models/label.rb
index 49c352cc239b811a7f8e772c00b9047d4d75f0a6..115f38c6dfe8d35c7e79d183027b04cc84e41b8f 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
@@ -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,18 @@ class Label < ActiveRecord::Base
def nullify_priority
self.priority = nil if priority.blank?
end
+
+ def sanitize_title(value)
+ unnescape_html_entities(Sanitize.clean(value.to_s))
+ end
+
+ def unnescape_html_entities(value)
+ value.to_s.gsub(/(>)|(<)|(&)/, Label::TABLE_FOR_ESCAPE_HTML_ENTITIES.invert)
+ end
+
+ TABLE_FOR_ESCAPE_HTML_ENTITIES = {
+ '&' => '&',
+ '<' => '<',
+ '>' => '>'
+ }
end
diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb
index dad2628651b480b74c98cdf759ac9df9d7159b44..f37f44a608e53c3aac5fa28a32a2e661f912afd5 100644
--- a/spec/models/label_spec.rb
+++ b/spec/models/label_spec.rb
@@ -32,21 +32,20 @@ describe Label, models: true do
it 'should validate title' do
expect(label).not_to allow_value('G,ITLAB').for(:title)
- expect(label).not_to allow_value('G?ITLAB').for(:title)
- expect(label).not_to allow_value('G&ITLAB').for(:title)
expect(label).not_to allow_value('').for(:title)
expect(label).to allow_value('GITLAB').for(:title)
expect(label).to allow_value('gitlab').for(:title)
+ expect(label).to allow_value('G?ITLAB').for(:title)
+ expect(label).to allow_value('G&ITLAB').for(:title)
expect(label).to allow_value("customer's request").for(:title)
end
end
- describe "#title" do
- let(:label) { create(:label, title: "<b>test</b>") }
-
- it "sanitizes title" do
- expect(label.title).to eq("test")
+ describe '#title' do
+ it 'sanitizes title' do
+ label = described_class.new(title: '<b>foo & bar?</b>')
+ expect(label.title).to eq('foo & bar?')
end
end
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 2cf130df328fc8e8b4c9d1fc6625eb26730fbdf2..6adccb4ebae9348f8d7e928b602a70b68267b2ef 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -482,12 +482,16 @@ describe API::API, api: true do
expect(response).to have_http_status(400)
end
- it 'should return 400 on invalid label names' do
+ it 'should allow special label names' do
post api("/projects/#{project.id}/issues", user),
title: 'new issue',
- labels: 'label, ?'
- expect(response).to have_http_status(400)
- expect(json_response['message']['labels']['?']['title']).to eq(['is invalid'])
+ labels: 'label, label?, label&foo, ?, &'
+ expect(response.status).to eq(201)
+ expect(json_response['labels']).to include 'label'
+ expect(json_response['labels']).to include 'label?'
+ expect(json_response['labels']).to include 'label&foo'
+ expect(json_response['labels']).to include '?'
+ expect(json_response['labels']).to include '&'
end
it 'should return 400 if title is too long' do
@@ -557,12 +561,17 @@ describe API::API, api: true do
expect(response).to have_http_status(404)
end
- it 'should return 400 on invalid label names' do
+ it 'should allow special label names' do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
title: 'updated title',
- labels: 'label, ?'
- expect(response).to have_http_status(400)
- expect(json_response['message']['labels']['?']['title']).to eq(['is invalid'])
+ labels: 'label, label?, label&foo, ?, &'
+
+ expect(response.status).to eq(200)
+ expect(json_response['labels']).to include 'label'
+ expect(json_response['labels']).to include 'label?'
+ expect(json_response['labels']).to include 'label&foo'
+ expect(json_response['labels']).to include '?'
+ expect(json_response['labels']).to include '&'
end
context 'confidential issues' do
@@ -627,21 +636,18 @@ describe API::API, api: true do
expect(json_response['labels']).to include 'bar'
end
- it 'should return 400 on invalid label names' do
- put api("/projects/#{project.id}/issues/#{issue.id}", user),
- labels: 'label, ?'
- expect(response).to have_http_status(400)
- expect(json_response['message']['labels']['?']['title']).to eq(['is invalid'])
- end
-
it 'should allow special label names' do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
- labels: 'label:foo, label-bar,label_bar,label/bar'
- expect(response).to have_http_status(200)
+ labels: 'label:foo, label-bar,label_bar,label/bar,label?bar,label&bar,?,&'
+ expect(response.status).to eq(200)
expect(json_response['labels']).to include 'label:foo'
expect(json_response['labels']).to include 'label-bar'
expect(json_response['labels']).to include 'label_bar'
expect(json_response['labels']).to include 'label/bar'
+ expect(json_response['labels']).to include 'label?bar'
+ expect(json_response['labels']).to include 'label&bar'
+ expect(json_response['labels']).to include '?'
+ expect(json_response['labels']).to include '&'
end
it 'should return 400 if title is too long' do
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index 0404cf31ff7c6b356c9ccec157e43e386eca5b50..63636b4a1b61a8cb0f1b270499ca34cc35ba7371 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/spec/requests/api/labels_spec.rb
@@ -35,10 +35,10 @@ describe API::API, api: true do
it 'should return created label when only required params' do
post api("/projects/#{project.id}/labels", user),
- name: 'Foo',
+ name: 'Foo & Bar',
color: '#FFAABB'
- expect(response).to have_http_status(201)
- expect(json_response['name']).to eq('Foo')
+ expect(response.status).to eq(201)
+ expect(json_response['name']).to eq('Foo & Bar')
expect(json_response['color']).to eq('#FFAABB')
expect(json_response['description']).to be_nil
end
@@ -71,7 +71,7 @@ describe API::API, api: true do
it 'should return 400 for invalid name' do
post api("/projects/#{project.id}/labels", user),
- name: '?',
+ name: ',',
color: '#FFAABB'
expect(response).to have_http_status(400)
expect(json_response['message']['title']).to eq(['is invalid'])
@@ -167,7 +167,7 @@ describe API::API, api: true do
it 'should return 400 for invalid name' do
put api("/projects/#{project.id}/labels", user),
name: 'label1',
- new_name: '?',
+ new_name: ',',
color: '#FFFFFF'
expect(response).to have_http_status(400)
expect(json_response['message']['title']).to eq(['is invalid'])
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 61e897edf8751afa22265df08cbe2eaed03125bb..5d81844fb84bd3ba2d6b64e46f188b888b53b33b 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -243,17 +243,19 @@ describe API::API, api: true do
expect(response).to have_http_status(400)
end
- it 'should return 400 on invalid label names' do
+ it 'should allow special label names' do
post api("/projects/#{project.id}/merge_requests", user),
title: 'Test merge_request',
source_branch: 'markdown',
target_branch: 'master',
author: user,
- labels: 'label, ?'
- expect(response).to have_http_status(400)
- expect(json_response['message']['labels']['?']['title']).to eq(
- ['is invalid']
- )
+ labels: 'label, label?, label&foo, ?, &'
+ expect(response.status).to eq(201)
+ expect(json_response['labels']).to include 'label'
+ expect(json_response['labels']).to include 'label?'
+ expect(json_response['labels']).to include 'label&foo'
+ expect(json_response['labels']).to include '?'
+ expect(json_response['labels']).to include '&'
end
context 'with existing MR' do
@@ -492,13 +494,17 @@ describe API::API, api: true do
expect(json_response['target_branch']).to eq('wiki')
end
- it 'should return 400 on invalid label names' do
+ it 'should allow special label names' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.id}",
user),
title: 'new issue',
- labels: 'label, ?'
- expect(response).to have_http_status(400)
- expect(json_response['message']['labels']['?']['title']).to eq(['is invalid'])
+ labels: 'label, label?, label&foo, ?, &'
+ expect(response.status).to eq(200)
+ expect(json_response['labels']).to include 'label'
+ expect(json_response['labels']).to include 'label?'
+ expect(json_response['labels']).to include 'label&foo'
+ expect(json_response['labels']).to include '?'
+ expect(json_response['labels']).to include '&'
end
end