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 + 1When 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 atrich_length,rich_posadvances 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 (&, <, ', 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.