fix(reports): escape test titles in the HTML report - #1250
Merged
Conversation
src/reports/html.sh had no escaping at all. A title is user text --
set_test_title takes anything and a data provider interpolates values into it
-- so it went straight into the table: a `<` corrupted it, a bare `&` was
invalid entity syntax, and `<script>` ran in whoever opened the report, which
for a CI artifact is a browser. Every other writer escapes; this was the one
that did not.
Rows were also joined into a temp file with `|` and split back on it, so a
title carrying one shifted every column and produced class="<fragment>", a CSS
class matching no rule, and the row lost its colour.
Separate the fields with US (0x1f) and escape & < > " in one awk pass.
awk rather than ${var//&/&}: a bare `&` in a bash replacement means "the
matched text" from 5.2 on while staying literal on 3.2 (#1096). The same rule
applies to gsub, hence \\&. The separator is passed in as a byte rather than
written \x1f, which is not POSIX awk (#1098). This is the approach
src/coverage/html_file.sh already uses for the coverage pages.
Closes #1249
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Background
Related #1249
src/reports/html.shhad no escaping at all. A test title is user text —bashunit::set_test_titletakes anything, and a data provider interpolates values into it — and it went straight into the markup:A
<corrupts the table, a bare&is invalid entity syntax, and a script tag runs in whoever opens the report — for a CI artifact, a browser. Every other writer escapes (JUnit, JSON, Markdown, GHA); this was the only one that did not.Same cause, second defect: rows were joined into a temp file with
|and split back on it, so a title carrying one shifted every column and emittedclass="after"— a CSS class matching no rule, so the row silently lost its status colour.💡 Changes
|&,<,>and"in oneawkpass over the collected rows — one fork per report, not per test&escaped, columns stay aligned, the title still reads as written, and a plain title is untouched so the escaping is not over-eagerawkrather than${var//&/&}for the reason already recorded in #1096: a bare&in a bash replacement means "the matched text" from 5.2 on while staying literal on 3.2. The same rule applies togsub, hence\\&, and the separator reachesawkas a byte rather than\x1f, which is not POSIX awk (#1098). This is the approachsrc/coverage/html_file.shalready uses for the coverage pages.