StringRangeMarker#safe_position_mapping overruns on literal & with no terminating ;

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Gitlab::StringRangeMarker#safe_position_mapping treats every & as the start of an HTML entity and scans forward to the next ; to collapse the entity to one position:

# lib/gitlab/string_range_marker.rb:86-92
if rich_char == '&'
  # Collect HTML entity positions
  entity_end = rich_pos
  entity_end += 1 while entity_end < rich_length && rich_line[entity_end] != ';'

  mapping[raw_pos] = (rich_pos..entity_end).to_a
  rich_pos = entity_end + 1

When a rich line contains a literal ampersand that is not part of an HTML entity (e.g. "a & b", "Tom & Jerry"), the scan (line 89) runs to the next unrelated ; or to the end of the line. Two failure modes result:

  • Bare & mid-line, later ;: the raw position for & maps to the whole span up to that unrelated ;.
  • Trailing & with no ;: the scan stops at rich_length, rich_pos advances past the end (line 92), and the outer loop breaks early (line 82). Every raw character after the & gets no mapping entry at all, silently truncating the marker.

Steps to reproduce

Mark a range on a rich line containing a bare & with no immediately following entity terminator, e.g. "a & b". Marker boundaries land at the wrong offsets; with a trailing &, characters after it are dropped from the mapping.

Expected

A literal & should map 1:1 like any other non-entity character. Only well-formed entities (&amp;, &lt;, &#39;, etc. terminated by ;) should be collapsed.

Suggested fix direction

Bound the entity scan (cap the lookahead, or validate the span matches an entity pattern like &(#\d+|#x[0-9a-f]+|\w+);) before collapsing. Add specs for a bare &, a & followed later on the line by an unrelated ;, and a trailing &.

Notes

Low severity: affects marker/highlight offset accuracy, not escaping or safety.

Edited by 🤖 GitLab Bot 🤖