From ab811b6ab929d3f220e060c15c49bc075d91e5f2 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 20 Jun 2016 18:33:01 -0300 Subject: [PATCH] Render references for labels that name contains ?, or & --- .../javascripts/gfm_auto_complete.js.coffee | 2 +- app/helpers/labels_helper.rb | 12 ++++- app/models/label.rb | 16 ++---- lib/banzai/filter/label_reference_filter.rb | 8 ++- spec/helpers/labels_helper_spec.rb | 6 +++ .../filter/label_reference_filter_spec.rb | 50 +++++++++++++++++++ 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 = { + '&' => '&', + '<' => '<', + '>' => '>' + } + # 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(/(>)|(<)|(&)/, 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 (?: (?\d+) | # Integer-based label ID, or (? - [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(/(>)|(<)|(&)/, Label::TABLE_FOR_ESCAPE_HTML_ENTITIES.invert) - end - - TABLE_FOR_ESCAPE_HTML_ENTITIES = { - '&' => '&', - '<' => '<', - '>' => '>' - } 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 & bar < zoo > boo é')).to eq 'foo & bar < zoo > boo é' + 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(\(\?gfm&\.\))) + end + + it 'ignores invalid label names' do + act = "Label #{Label.reference_prefix}#{label.name.reverse}" + exp = "Label #{Label.reference_prefix}&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(\(gfm & references\?\.\))) + end + + it 'ignores invalid label names' do + act = %(Label #{Label.reference_prefix}"#{label.name.reverse}") + exp = %(Label #{Label.reference_prefix}"?secnerefer & 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'