diff options
-rw-r--r-- | app/models/label.rb | 10 | ||||
-rw-r--r-- | lib/gitlab/markdown/label_reference_filter.rb | 3 | ||||
-rw-r--r-- | spec/lib/gitlab/markdown/label_reference_filter_spec.rb | 54 | ||||
-rw-r--r-- | spec/models/label_spec.rb | 19 |
4 files changed, 33 insertions, 53 deletions
diff --git a/app/models/label.rb b/app/models/label.rb index 8980049cef8..230631b5180 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -41,16 +41,14 @@ class Label < ActiveRecord::Base end # Pattern used to extract label references from text - # - # TODO (rspeicher): Limit to double quotes (meh) or disallow single quotes in label names (bad). def self.reference_pattern %r{ #{reference_prefix} (?: - (?<label_id>\d+) | # Integer-based label ID, or + (?<label_id>\d+) | # Integer-based label ID, or (?<label_name> - [A-Za-z0-9_-]+ | # String-based single-word label title - ['"][^&\?,]+['"] # 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 @@ -70,7 +68,7 @@ class Label < ActiveRecord::Base # # Returns a String def to_reference(format = :id) - if format == :name + if format == :name && !name.include?('"') %(#{self.class.reference_prefix}"#{name}") else "#{self.class.reference_prefix}#{id}" diff --git a/lib/gitlab/markdown/label_reference_filter.rb b/lib/gitlab/markdown/label_reference_filter.rb index 9f8c85b7012..e022ca69c91 100644 --- a/lib/gitlab/markdown/label_reference_filter.rb +++ b/lib/gitlab/markdown/label_reference_filter.rb @@ -72,8 +72,7 @@ module Gitlab # Returns a Hash. def label_params(id, name) if name - # TODO (rspeicher): Don't strip single quotes if we decide to only use double quotes for surrounding. - { name: name.tr('\'"', '') } + { name: name.tr('"', '') } else { id: id } end diff --git a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb index 250a44d575d..41987f57bca 100644 --- a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb @@ -100,52 +100,26 @@ module Gitlab::Markdown end context 'String-based multi-word references in quotes' do - let(:label) { create(:label, name: 'gfm references', project: project) } + let(:label) { create(:label, name: 'gfm references', project: project) } + let(:reference) { label.to_reference(:name) } - context 'in single quotes' do - let(:reference) { "#{Label.reference_prefix}'#{label.name}'" } - - it 'links to a valid reference' do - doc = 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 = filter("Label (#{reference}.)") - expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\.\))) - end - - it 'ignores invalid label names' do - exp = act = "Label #{Label.reference_prefix}'#{label.name.reverse}'" + it 'links to a valid reference' do + doc = filter("See #{reference}") - expect(filter(act).to_html).to eq exp - end + 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 - context 'in double quotes' do - let(:reference) { %(#{Label.reference_prefix}"#{label.name}") } - - it 'links to a valid reference' do - doc = 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 = filter("Label (#{reference}.)") - expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\.\))) - end + it 'links with adjacent text' do + doc = filter("Label (#{reference}.)") + expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\.\))) + end - it 'ignores invalid label names' do - exp = act = %(Label #{Label.reference_prefix}"#{label.name.reverse}") + it 'ignores invalid label names' do + exp = act = %(Label #{Label.reference_prefix}"#{label.name.reverse}") - expect(filter(act).to_html).to eq exp - end + expect(filter(act).to_html).to eq exp end end diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index a13f9ac926c..6518213d71c 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -55,13 +55,22 @@ describe Label do end describe '#to_reference' do - it 'returns a String reference to the object' do - expect(label.to_reference).to eq "~#{label.id}" - expect(label.to_reference(double)).to eq "~#{label.id}" + context 'using id' do + it 'returns a String reference to the object' do + expect(label.to_reference).to eq "~#{label.id}" + expect(label.to_reference(double('project'))).to eq "~#{label.id}" + end end - it 'returns a String reference to the object using its name' do - expect(label.to_reference(:name)).to eq %(~"#{label.name}") + context 'using name' do + it 'returns a String reference to the object' do + expect(label.to_reference(:name)).to eq %(~"#{label.name}") + end + + it 'uses id when name contains double quote' do + label = create(:label, name: %q{"irony"}) + expect(label.to_reference(:name)).to eq "~#{label.id}" + end end end end |