From c271fdc80fd17e1931a1d912e231bec2b8e3d098 Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Tue, 15 Aug 2017 07:09:05 +0200 Subject: Don't escape html entities when rich == raw line --- changelogs/unreleased/36041-notification-title.yml | 4 +++ lib/gitlab/string_range_marker.rb | 34 ++++++++++++------- spec/helpers/diff_helper_spec.rb | 8 ++--- .../diff/inline_diff_markdown_marker_spec.rb | 6 ++-- spec/lib/gitlab/diff/inline_diff_marker_spec.rb | 14 ++++---- spec/lib/gitlab/string_range_marker_spec.rb | 39 ++++++++++++---------- 6 files changed, 62 insertions(+), 43 deletions(-) create mode 100644 changelogs/unreleased/36041-notification-title.yml diff --git a/changelogs/unreleased/36041-notification-title.yml b/changelogs/unreleased/36041-notification-title.yml new file mode 100644 index 00000000000..7c5e0a0cd0d --- /dev/null +++ b/changelogs/unreleased/36041-notification-title.yml @@ -0,0 +1,4 @@ +--- +title: Don't escape html entities in InlineDiffMarkdownMarker +merge_request: +author: diff --git a/lib/gitlab/string_range_marker.rb b/lib/gitlab/string_range_marker.rb index 94fba0a221a..11aeec1ebfa 100644 --- a/lib/gitlab/string_range_marker.rb +++ b/lib/gitlab/string_range_marker.rb @@ -1,21 +1,31 @@ module Gitlab class StringRangeMarker - attr_accessor :raw_line, :rich_line - - def initialize(raw_line, rich_line = raw_line) - @raw_line = raw_line - @rich_line = ERB::Util.html_escape(rich_line) + attr_accessor :raw_line, :rich_line, :html_escaped + + def initialize(raw_line, rich_line = nil) + @raw_line = raw_line.dup + if rich_line.nil? + @rich_line = raw_line.dup + @html_escaped = false + else + @rich_line = ERB::Util.html_escape(rich_line) + @html_escaped = true + end end def mark(marker_ranges) return rich_line unless marker_ranges - rich_marker_ranges = [] - marker_ranges.each do |range| - # Map the inline-diff range based on the raw line to character positions in the rich line - rich_positions = position_mapping[range].flatten - # Turn the array of character positions into ranges - rich_marker_ranges.concat(collapse_ranges(rich_positions)) + if html_escaped + rich_marker_ranges = [] + marker_ranges.each do |range| + # Map the inline-diff range based on the raw line to character positions in the rich line + rich_positions = position_mapping[range].flatten + # Turn the array of character positions into ranges + rich_marker_ranges.concat(collapse_ranges(rich_positions)) + end + else + rich_marker_ranges = marker_ranges end offset = 0 @@ -31,7 +41,7 @@ module Gitlab offset += text.length - original_text.length end - rich_line.html_safe + @html_escaped ? rich_line.html_safe : rich_line end private diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index f81a9b6492c..0deea0ff6a3 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -135,10 +135,10 @@ describe DiffHelper do it "returns strings with marked inline diffs" do marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line) - expect(marked_old_line).to eq(%q{abc 'def'}) - expect(marked_old_line).to be_html_safe - expect(marked_new_line).to eq(%q{abc "def"}) - expect(marked_new_line).to be_html_safe + expect(marked_old_line).to eq(%q{abc 'def'}) + expect(marked_old_line).not_to be_html_safe + expect(marked_new_line).to eq(%q{abc "def"}) + expect(marked_new_line).not_to be_html_safe end end diff --git a/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb b/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb index 046b096e366..7e17437fa2a 100644 --- a/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb @@ -6,9 +6,9 @@ describe Gitlab::Diff::InlineDiffMarkdownMarker do let(:inline_diffs) { [2..5] } let(:subject) { described_class.new(raw).mark(inline_diffs, mode: :deletion) } - it 'marks the range' do - expect(subject).to eq("ab{-c 'd-}ef'") - expect(subject).to be_html_safe + it 'does not escape html etities and marks the range' do + expect(subject).to eq("ab{-c 'd-}ef'") + expect(subject).not_to be_html_safe end end end diff --git a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb index c3bf34c24ae..7296bbf5df3 100644 --- a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb @@ -2,11 +2,13 @@ require 'spec_helper' describe Gitlab::Diff::InlineDiffMarker do describe '#mark' do + let(:inline_diffs) { [2..5] } + let(:raw) { "abc 'def'" } + + subject { described_class.new(raw, rich).mark(inline_diffs) } + context "when the rich text is html safe" do - let(:raw) { "abc 'def'" } let(:rich) { %{abc 'def'}.html_safe } - let(:inline_diffs) { [2..5] } - let(:subject) { described_class.new(raw, rich).mark(inline_diffs) } it 'marks the range' do expect(subject).to eq(%{abc 'def'}) @@ -15,12 +17,10 @@ describe Gitlab::Diff::InlineDiffMarker do end context "when the text text is not html safe" do - let(:raw) { "abc 'def'" } - let(:inline_diffs) { [2..5] } - let(:subject) { described_class.new(raw).mark(inline_diffs) } + let(:rich) { "abc 'def' differs" } it 'marks the range' do - expect(subject).to eq(%{abc 'def'}) + expect(subject).to eq(%{abc 'def' differs}) expect(subject).to be_html_safe end end diff --git a/spec/lib/gitlab/string_range_marker_spec.rb b/spec/lib/gitlab/string_range_marker_spec.rb index abeaa7f0ddb..6bc02459dbd 100644 --- a/spec/lib/gitlab/string_range_marker_spec.rb +++ b/spec/lib/gitlab/string_range_marker_spec.rb @@ -2,34 +2,39 @@ require 'spec_helper' describe Gitlab::StringRangeMarker do describe '#mark' do + def mark_diff(rich = nil) + raw = 'abc ' + inline_diffs = [2..5] + + described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:| + "LEFT#{text}RIGHT" + end + end + context "when the rich text is html safe" do - let(:raw) { "abc " } let(:rich) { %{abc <def>}.html_safe } - let(:inline_diffs) { [2..5] } - subject do - described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:| - "LEFT#{text}RIGHT" - end - end it 'marks the inline diffs' do - expect(subject).to eq(%{abLEFTcRIGHTLEFT RIGHTLEFT<dRIGHTef>}) - expect(subject).to be_html_safe + expect(mark_diff(rich)).to eq(%{abLEFTcRIGHTLEFT RIGHTLEFT<dRIGHTef>}) + expect(mark_diff(rich)).to be_html_safe end end context "when the rich text is not html safe" do - let(:raw) { "abc " } - let(:inline_diffs) { [2..5] } - subject do - described_class.new(raw).mark(inline_diffs) do |text, left:, right:| - "LEFT#{text}RIGHT" + context 'when rich text equals raw text' do + it 'marks the inline diffs' do + expect(mark_diff).to eq(%{abLEFTc }) + expect(mark_diff).not_to be_html_safe end end - it 'marks the inline diffs' do - expect(subject).to eq(%{abLEFTc <dRIGHTef>}) - expect(subject).to be_html_safe + context 'when rich text doeas not equal raw text' do + let(:rich) { "abc differs" } + + it 'marks the inline diffs' do + expect(mark_diff(rich)).to eq(%{abLEFTc <dRIGHTef> differs}) + expect(mark_diff(rich)).to be_html_safe + end end end end -- cgit v1.2.1