Properly handle HTML entities with inline diffs

5 files changed, 24 insertions, 20 deletions

diff --git a/lib/gitlab/diff/inline_diff.rb b/lib/gitlab/diff/inline_diff.rb
index e5986fd69e2..b8a61ad6115 100644
--- a/lib/gitlab/diff/inline_diff.rb
+++ b/lib/gitlab/diff/inline_diff.rb
@@ -16,15 +16,12 @@ module Gitlab
           old_line = @lines[old_index]
           new_line = @lines[new_index]
 
-          suffixless_old_line = old_line[1..-1]
-          suffixless_new_line = new_line[1..-1]
-
           # Skip inline diff if empty line was replaced with content
-          next if suffixless_old_line == ""
+          next if old_line[1..-1] == ""
 
-          # Add one, because this is based on the suffixless version
-          lcp = longest_common_prefix(suffixless_old_line, suffixless_new_line) + 1
-          lcs = longest_common_suffix(suffixless_old_line, suffixless_new_line)
+          # Add one, because this is based on the prefixless version
+          lcp = longest_common_prefix(old_line[1..-1], new_line[1..-1]) + 1
+          lcs = longest_common_suffix(old_line[lcp..-1], new_line[lcp..-1])
 
           old_diff_range = lcp..(old_line.length - lcs - 1)
           new_diff_range = lcp..(new_line.length - lcs - 1)
diff --git a/lib/gitlab/diff/inline_diff_marker.rb b/lib/gitlab/diff/inline_diff_marker.rb
index 8998ccba5ce..c31149d374e 100644
--- a/lib/gitlab/diff/inline_diff_marker.rb
+++ b/lib/gitlab/diff/inline_diff_marker.rb
@@ -12,7 +12,7 @@ module Gitlab
         offset = 0
         line_inline_diffs.each do |inline_diff_range|
           # Map the inline-diff range based on the raw line to character positions in the rich line
-          inline_diff_positions = position_mapping[inline_diff_range]
+          inline_diff_positions = position_mapping[inline_diff_range].flatten
           # Turn the array of character positions into ranges
           marker_ranges = collapse_ranges(inline_diff_positions)
 
@@ -47,7 +47,19 @@ module Gitlab
               rich_char = rich_line[rich_pos]
             end
 
-            mapping[raw_pos] = rich_pos
+            # multi-char HTML entities in the rich line correspond to a single character in the raw line
+            if rich_char == '&'
+              multichar_mapping = [rich_pos]
+              until rich_char == ';'
+                rich_pos += 1
+                multichar_mapping << rich_pos
+                rich_char = rich_line[rich_pos]
+              end
+
+              mapping[raw_pos] = multichar_mapping
+            else
+              mapping[raw_pos] = rich_pos
+            end
 
             rich_pos += 1
           end
diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb
index 2c0a866e8ba..6c8a1fc6d6f 100644
--- a/lib/gitlab/diff/parser.rb
+++ b/lib/gitlab/diff/parser.rb
@@ -14,7 +14,7 @@ module Gitlab
         @lines.each do |line|
           next if filename?(line)
 
-          full_line = html_escape(line.gsub(/\n/, ''))
+          full_line = line.gsub(/\n/, '')
 
           if line.match(/^@@ -/)
             type = "match"
@@ -67,11 +67,6 @@ module Gitlab
           nil
         end
       end
-
-      def html_escape(str)
-        replacements = { '&' => '&amp;', '>' => '&gt;', '<' => '&lt;', '"' => '&quot;', "'" => '&#39;' }
-        str.gsub(/[&"'><]/, replacements)
-      end
     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 5d6aea17509..6f3276a8b53 100644
--- a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb
+++ b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb
@@ -2,14 +2,14 @@ require 'spec_helper'
 
 describe Gitlab::Diff::InlineDiffMarker, lib: true do
   describe '#inline_diffs' do
-    let(:raw)  { "abc def" }
-    let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">def</span>} }
-    let(:inline_diffs) { [2..4] }
+    let(:raw)  { "abc 'def'" }
+    let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">&#39;def&#39;</span>} }
+    let(:inline_diffs) { [2..5] }
 
     let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw, rich).mark(inline_diffs) }
 
     it 'marks the inline diffs' do
-      expect(subject).to eq(%{<span class="abc">ab<span class='idiff'>c</span></span><span class="space"><span class='idiff'> </span></span><span class="def"><span class='idiff'>d</span>ef</span>})
+      expect(subject).to eq(%{<span class="abc">ab<span class='idiff'>c</span></span><span class="space"><span class='idiff'> </span></span><span class="def"><span class='idiff'>&#39;d</span>ef&#39;</span>})
     end
   end
 end
diff --git a/spec/lib/gitlab/diff/parser_spec.rb b/spec/lib/gitlab/diff/parser_spec.rb
index ba577bd28e5..fe0dea77909 100644
--- a/spec/lib/gitlab/diff/parser_spec.rb
+++ b/spec/lib/gitlab/diff/parser_spec.rb
@@ -86,7 +86,7 @@ eos
         it { expect(line.type).to eq(nil) }
         it { expect(line.old_pos).to eq(24) }
         it { expect(line.new_pos).to eq(31) }
-        it { expect(line.text).to eq('       @cmd_output &lt;&lt; stderr.read') }
+        it { expect(line.text).to eq('       @cmd_output << stderr.read') }
       end
     end
   end