Allow '?', or '&' for label titles

parent cfd5870b
...@@ -32,9 +32,9 @@ class @LabelsSelect ...@@ -32,9 +32,9 @@ class @LabelsSelect
if issueUpdateURL if issueUpdateURL
labelHTMLTemplate = _.template( labelHTMLTemplate = _.template(
'<% _.each(labels, function(label){ %> '<% _.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 %>;"> <span class="label has-tooltip color-label" title="<%- label.description %>" style="background-color: <%- label.color %>; color: <%- label.text_color %>;">
<%- label.title %> <%= label.title %>
</span> </span>
</a> </a>
<% }); %>' <% }); %>'
......
...@@ -20,10 +20,10 @@ class Label < ActiveRecord::Base ...@@ -20,10 +20,10 @@ class Label < ActiveRecord::Base
validates :color, color: true, allow_blank: false validates :color, color: true, allow_blank: false
validates :project, presence: true, unless: Proc.new { |service| service.template? } 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, validates :title,
presence: true, presence: true,
format: { with: /\A[^&\?,]+\z/ }, format: { with: /\A[^,]+\z/ },
uniqueness: { scope: :project_id } uniqueness: { scope: :project_id }
before_save :nullify_priority before_save :nullify_priority
...@@ -114,7 +114,7 @@ class Label < ActiveRecord::Base ...@@ -114,7 +114,7 @@ class Label < ActiveRecord::Base
end end
def title=(value) def title=(value)
write_attribute(:title, Sanitize.clean(value.to_s)) if value.present? write_attribute(:title, sanitize_title(value)) if value.present?
end end
private private
...@@ -132,4 +132,18 @@ class Label < ActiveRecord::Base ...@@ -132,4 +132,18 @@ class Label < ActiveRecord::Base
def nullify_priority def nullify_priority
self.priority = nil if priority.blank? self.priority = nil if priority.blank?
end end
def sanitize_title(value)
unnescape_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 end
...@@ -32,21 +32,20 @@ describe Label, models: true do ...@@ -32,21 +32,20 @@ describe Label, models: true do
it 'should validate title' 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('G&ITLAB').for(:title)
expect(label).not_to allow_value('').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('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) expect(label).to allow_value("customer's request").for(:title)
end end
end end
describe "#title" do describe '#title' do
let(:label) { create(:label, title: "<b>test</b>") } it 'sanitizes title' do
label = described_class.new(title: '<b>foo & bar?</b>')
it "sanitizes title" do expect(label.title).to eq('foo & bar?')
expect(label.title).to eq("test")
end end
end end
......
...@@ -482,12 +482,16 @@ describe API::API, api: true do ...@@ -482,12 +482,16 @@ describe API::API, api: true do
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
end end
it 'should return 400 on invalid label names' do it 'should allow special label names' do
post api("/projects/#{project.id}/issues", user), post api("/projects/#{project.id}/issues", user),
title: 'new issue', title: 'new issue',
labels: 'label, ?' labels: 'label, label?, label&foo, ?, &'
expect(response).to have_http_status(400) expect(response.status).to eq(201)
expect(json_response['message']['labels']['?']['title']).to eq(['is invalid']) 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
it 'should return 400 if title is too long' do it 'should return 400 if title is too long' do
...@@ -557,12 +561,17 @@ describe API::API, api: true do ...@@ -557,12 +561,17 @@ describe API::API, api: true do
expect(response).to have_http_status(404) expect(response).to have_http_status(404)
end 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), put api("/projects/#{project.id}/issues/#{issue.id}", user),
title: 'updated title', title: 'updated title',
labels: 'label, ?' labels: 'label, label?, label&foo, ?, &'
expect(response).to have_http_status(400)
expect(json_response['message']['labels']['?']['title']).to eq(['is invalid']) 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
context 'confidential issues' do context 'confidential issues' do
...@@ -627,21 +636,18 @@ describe API::API, api: true do ...@@ -627,21 +636,18 @@ describe API::API, api: true do
expect(json_response['labels']).to include 'bar' expect(json_response['labels']).to include 'bar'
end 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 it 'should allow special label names' do
put api("/projects/#{project.id}/issues/#{issue.id}", user), put api("/projects/#{project.id}/issues/#{issue.id}", user),
labels: 'label:foo, label-bar,label_bar,label/bar' labels: 'label:foo, label-bar,label_bar,label/bar,label?bar,label&bar,?,&'
expect(response).to have_http_status(200) expect(response.status).to eq(200)
expect(json_response['labels']).to include 'label:foo' 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 '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 end
it 'should return 400 if title is too long' do it 'should return 400 if title is too long' do
......
...@@ -35,10 +35,10 @@ describe API::API, api: true do ...@@ -35,10 +35,10 @@ describe API::API, api: true do
it 'should return created label when only required params' do it 'should return created label when only required params' do
post api("/projects/#{project.id}/labels", user), post api("/projects/#{project.id}/labels", user),
name: 'Foo', name: 'Foo & Bar',
color: '#FFAABB' color: '#FFAABB'
expect(response).to have_http_status(201) expect(response.status).to eq(201)
expect(json_response['name']).to eq('Foo') expect(json_response['name']).to eq('Foo & Bar')
expect(json_response['color']).to eq('#FFAABB') expect(json_response['color']).to eq('#FFAABB')
expect(json_response['description']).to be_nil expect(json_response['description']).to be_nil
end end
...@@ -71,7 +71,7 @@ describe API::API, api: true do ...@@ -71,7 +71,7 @@ describe API::API, api: true do
it 'should return 400 for invalid name' do it 'should return 400 for invalid name' do
post api("/projects/#{project.id}/labels", user), post api("/projects/#{project.id}/labels", user),
name: '?', name: ',',
color: '#FFAABB' color: '#FFAABB'
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']['title']).to eq(['is invalid']) expect(json_response['message']['title']).to eq(['is invalid'])
...@@ -167,7 +167,7 @@ describe API::API, api: true do ...@@ -167,7 +167,7 @@ describe API::API, api: true do
it 'should return 400 for invalid name' do it 'should return 400 for invalid name' do
put api("/projects/#{project.id}/labels", user), put api("/projects/#{project.id}/labels", user),
name: 'label1', name: 'label1',
new_name: '?', new_name: ',',
color: '#FFFFFF' color: '#FFFFFF'
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']['title']).to eq(['is invalid']) expect(json_response['message']['title']).to eq(['is invalid'])
......
...@@ -243,17 +243,19 @@ describe API::API, api: true do ...@@ -243,17 +243,19 @@ describe API::API, api: true do
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
end 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), post api("/projects/#{project.id}/merge_requests", user),
title: 'Test merge_request', title: 'Test merge_request',
source_branch: 'markdown', source_branch: 'markdown',
target_branch: 'master', target_branch: 'master',
author: user, author: user,
labels: 'label, ?' labels: 'label, label?, label&foo, ?, &'
expect(response).to have_http_status(400) expect(response.status).to eq(201)
expect(json_response['message']['labels']['?']['title']).to eq( expect(json_response['labels']).to include 'label'
['is invalid'] 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
context 'with existing MR' do context 'with existing MR' do
...@@ -492,13 +494,17 @@ describe API::API, api: true do ...@@ -492,13 +494,17 @@ describe API::API, api: true do
expect(json_response['target_branch']).to eq('wiki') expect(json_response['target_branch']).to eq('wiki')
end 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}", put api("/projects/#{project.id}/merge_requests/#{merge_request.id}",
user), user),
title: 'new issue', title: 'new issue',
labels: 'label, ?' labels: 'label, label?, label&foo, ?, &'
expect(response).to have_http_status(400) expect(response.status).to eq(200)
expect(json_response['message']['labels']['?']['title']).to eq(['is invalid']) 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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment