From 963fbeffc887a7a7acfb0e6dcd8e9a1c9f0deda7 Mon Sep 17 00:00:00 2001 From: gschreiber Date: Sat, 11 Mar 2023 11:12:21 +0100 Subject: [PATCH 1/3] html escape dashboard results --- .eslintrc.json | 3 +- src/dashboard.ts | 104 +++++++++++++++++++++++++++++++++++++++++++++ src/escape_html.ts | 11 +++++ src/index.ts | 88 +------------------------------------- test/dashboard.ts | 51 ++++++++++++++++++++++ 5 files changed, 169 insertions(+), 88 deletions(-) create mode 100644 src/dashboard.ts create mode 100644 src/escape_html.ts create mode 100644 test/dashboard.ts diff --git a/.eslintrc.json b/.eslintrc.json index f92e594..31fc7d9 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -60,7 +60,8 @@ "semi": "off", "@typescript-eslint/semi": ["error", "never"], "@typescript-eslint/type-annotation-spacing": "error", - "@typescript-eslint/unbound-method": "error" + "@typescript-eslint/unbound-method": "error", + "i18n-text/no-en": "off" // allow English string literals }, "env": { "node": true, diff --git a/src/dashboard.ts b/src/dashboard.ts new file mode 100644 index 0000000..dcf5db6 --- /dev/null +++ b/src/dashboard.ts @@ -0,0 +1,104 @@ +import escapeHTML from "./escape_html" +import { TestResult, TestStatus } from "./test_parser" + +const dashboardUrl = "https://svg.test-summary.com/dashboard.svg" +const passIconUrl = "https://svg.test-summary.com/icon/pass.svg?s=12" +const failIconUrl = "https://svg.test-summary.com/icon/fail.svg?s=12" +const skipIconUrl = "https://svg.test-summary.com/icon/skip.svg?s=12" +// not used: const noneIconUrl = 'https://svg.test-summary.com/icon/none.svg?s=12' + +const unnamedTestCase = "" + +const footer = `This test report was produced by the test-summary action.  Made with ❤️ in Cambridge.` + +export function dashboardSummary(result: TestResult): string { + const count = result.counts + let summary = "" + + if (count.passed > 0) { + summary += `${count.passed} passed` + } + if (count.failed > 0) { + summary += `${summary ? ", " : ""}${count.failed} failed` + } + if (count.skipped > 0) { + summary += `${summary ? ", " : ""}${count.skipped} skipped` + } + + return `${summary}` +} + +export function dashboardResults(result: TestResult, show: number): string { + let table = "" + let count = 0 + + table += `` + + for (const suite of result.suites) { + for (const testcase of suite.cases) { + if (show !== 0 && (show & testcase.status) === 0) { + continue + } + + table += "\n" + + count++ + } + } + + table += `` + table += "
${statusTitle(show)}:
" + + const icon = statusIcon(testcase.status) + if (icon) { + table += icon + table += "  " + } + + table += escapeHTML(testcase.name || unnamedTestCase) + + if (testcase.description) { + table += ": " + table += escapeHTML(testcase.description) + } + + if (testcase.details) { + table += "
"
+                table += escapeHTML(testcase.details)
+                table += "
" + } + + table += "
${footer}
" + + if (count === 0) { + return "" + } + + return table +} + +function statusTitle(status: TestStatus): string { + switch (status) { + case TestStatus.Fail: + return "Test failures" + case TestStatus.Skip: + return "Skipped tests" + case TestStatus.Pass: + return "Passing tests" + default: + return "Test results" + } +} + +function statusIcon(status: TestStatus): string | undefined { + switch (status) { + case TestStatus.Pass: + return `` + case TestStatus.Fail: + return `` + case TestStatus.Skip: + return `` + default: + return + } +} diff --git a/src/escape_html.ts b/src/escape_html.ts new file mode 100644 index 0000000..1f8955e --- /dev/null +++ b/src/escape_html.ts @@ -0,0 +1,11 @@ +const lookup: Record = { + "&": "&", + '"': """, + "'": "'", + "<": "<", + ">": ">" +} + +export default function escapeHTML(s: string): string { + return s.replace(/[&"'<>]/g, c => lookup[c]) +} diff --git a/src/index.ts b/src/index.ts index af5c015..644afa3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,14 +4,7 @@ import * as core from "@actions/core" import * as glob from "glob-promise" import { TestResult, TestStatus, parseFile } from "./test_parser" - -const dashboardUrl = 'https://svg.test-summary.com/dashboard.svg' -const passIconUrl = 'https://svg.test-summary.com/icon/pass.svg?s=12' -const failIconUrl = 'https://svg.test-summary.com/icon/fail.svg?s=12' -const skipIconUrl = 'https://svg.test-summary.com/icon/skip.svg?s=12' -const noneIconUrl = 'https://svg.test-summary.com/icon/none.svg?s=12' - -const footer = `This test report was produced by the test-summary action.  Made with ❤️ in Cambridge.` +import { dashboardResults, dashboardSummary } from "./dashboard" async function run(): Promise { try { @@ -131,83 +124,4 @@ async function run(): Promise { } } -function dashboardSummary(result: TestResult) { - const count = result.counts - let summary = "" - - if (count.passed > 0) { - summary += `${count.passed} passed` - } - if (count.failed > 0) { - summary += `${summary ? ', ' : '' }${count.failed} failed` - } - if (count.skipped > 0) { - summary += `${summary ? ', ' : '' }${count.skipped} skipped` - } - - return `${summary}` -} - -function dashboardResults(result: TestResult, show: number) { - let table = "" - let count = 0 - let title: string - - if (show == TestStatus.Fail) { - title = "Test failures" - } else if (show === TestStatus.Skip) { - title = "Skipped tests" - } else if (show === TestStatus.Pass) { - title = "Passing tests" - } else { - title = "Test results" - } - - table += `` - - for (const suite of result.suites) { - for (const testcase of suite.cases) { - if (show != 0 && (show & testcase.status) == 0) { - continue - } - - table += "\n" - - count++ - } - } - - table += `` - table += "
${title}:
" - - if (testcase.status == TestStatus.Pass) { - table += `  ` - } else if (testcase.status == TestStatus.Fail) { - table += `  ` - } else if (testcase.status == TestStatus.Skip) { - table += `  ` - } - - table += testcase.name - - if (testcase.description) { - table += ": " - table += testcase.description - } - - if (testcase.details) { - table += "
"
-                table += testcase.details
-                table += "
" - } - - table += "
${footer}
" - - if (count == 0) { - return "" - } - - return table -} - run() diff --git a/test/dashboard.ts b/test/dashboard.ts new file mode 100644 index 0000000..8589832 --- /dev/null +++ b/test/dashboard.ts @@ -0,0 +1,51 @@ +import { expect } from "chai" + +import { TestStatus, TestResult } from "../src/test_parser" +import { dashboardResults } from "../src/dashboard" + +describe("dashboard", async () => { + it("escapes HTML entities", async () => { + const result: TestResult = { + counts: { passed: 0, failed: 2, skipped: 0 }, + suites: [ + { + cases: [ + { + status: TestStatus.Fail, + name: "name escaped ", // "<" and ">" require escaping + description: "description escaped \"properly\"", // double quotes require escaping + }, + { + status: TestStatus.Fail, + name: "another name escaped 'properly'", // single quotes require escaping + description: "another description escaped & properly", // ampersand requires escaping + } + ] + } + ] + } + const actual = dashboardResults(result, TestStatus.Fail) + expect(actual).contains("name escaped <properly>") + expect(actual).contains("description escaped "properly"") + expect(actual).contains("another name escaped 'properly'") + expect(actual).contains("another description escaped & properly") + }) + + it("uses for test cases without name", async () => { + const result: TestResult = { + counts: { passed: 0, failed: 1, skipped: 0 }, + suites: [ + { + cases: [ + { + status: TestStatus.Fail, + // <-- no name + } + ] + } + ] + } + const actual = dashboardResults(result, TestStatus.Fail) + expect(actual).contains("<no name>") + }) +}) From 696de5a9aecd9798f55b11480fe72e165d8dacec Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 Dec 2023 09:41:51 +0000 Subject: [PATCH 2/3] Massage HTML output for GitHub's parser GitHub's parser treats '
' at the beginning of a line specially and
will change escaping when it sees it. Cope with that reality.
---
 src/dashboard.ts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/dashboard.ts b/src/dashboard.ts
index dcf5db6..90bf997 100644
--- a/src/dashboard.ts
+++ b/src/dashboard.ts
@@ -56,7 +56,8 @@ export function dashboardResults(result: TestResult, show: number): string {
             }
 
             if (testcase.details) {
-                table += "
"
+                table += "
\n" + table += "
"
                 table += escapeHTML(testcase.details)
                 table += "
" } From 231278887da29fc8e75e9abaa62e8f7eae8aba46 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 Dec 2023 09:53:38 +0000 Subject: [PATCH 3/3] Ensure we escape sequences of different entities In a naive, multi-pass entity replacement (eg, replace all `&` with `&`, replace all `<` with `<`) the replacement order is important. (You must replace `&` with `&` first, lest you replace `<` with `<` then replace `<` with `&lt;`.) The `escapeHTML` function is a single-pass replacement of each entity at a time, so is not vulnerable to such a failure mode, but add a test to avoid regressions. --- test/dashboard.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/dashboard.ts b/test/dashboard.ts index 8589832..ede9b51 100644 --- a/test/dashboard.ts +++ b/test/dashboard.ts @@ -19,6 +19,11 @@ describe("dashboard", async () => { status: TestStatus.Fail, name: "another name escaped 'properly'", // single quotes require escaping description: "another description escaped & properly", // ampersand requires escaping + }, + { + status: TestStatus.Fail, + name: "entities ' are & escaped < in > proper & order", + description: "order is important in a multi-pass replacement", } ] } @@ -29,6 +34,7 @@ describe("dashboard", async () => { expect(actual).contains("description escaped "properly"") expect(actual).contains("another name escaped 'properly'") expect(actual).contains("another description escaped & properly") + expect(actual).contains("entities ' are & escaped < in > proper & order") }) it("uses for test cases without name", async () => {