Escape the values interpolated into the one markup-carrying message
CI / full-gate (push) Successful in 2m37s
CI / full-gate (push) Successful in 2m37s
This commit is contained in:
@@ -147,8 +147,9 @@ them. Revisit only if the stated reason stops holding.
|
|||||||
branding, or a menu `rename` be either a key or plain text without a second field or a migration.
|
branding, or a menu `rename` be either a key or plain text without a second field or a migration.
|
||||||
Don't "fix" it into a loud failure: a manifest with plain labels must keep working.
|
Don't "fix" it into a loud failure: a manifest with plain labels must keep working.
|
||||||
- **`t()` returns raw text; the view escapes it.** Messages go through `<%= %>` like any other value,
|
- **`t()` returns raw text; the view escapes it.** Messages go through `<%= %>` like any other value,
|
||||||
so nothing is double-escaped; a message carrying markup uses `<%- %>` and must not interpolate
|
so nothing is double-escaped; a message carrying markup uses `<%- %>`, and then its `{{vars}}` are
|
||||||
untrusted data. Don't move escaping into `t()`.
|
escaped at the call site (see `views/partials/pagination.ejs`). Don't move escaping into `t()` —
|
||||||
|
every other value in a view would then be the odd one out.
|
||||||
- **CI docker logins share the runner host's Docker config.** The act_runner is host-mode, so
|
- **CI docker logins share the runner host's Docker config.** The act_runner is host-mode, so
|
||||||
`docker login`/`logout` in the workflows mutate one shared `~/.docker/config.json`:
|
`docker login`/`logout` in the workflows mutate one shared `~/.docker/config.json`:
|
||||||
concurrent jobs can race (one job's logout can 401 another's push — recover by re-running),
|
concurrent jobs can race (one job's logout can 401 another's push — recover by re-running),
|
||||||
|
|||||||
@@ -998,8 +998,8 @@ Three rules worth knowing:
|
|||||||
plain text — `nav: [{ label: "shop.title" }]` is translated, `label: "Shop"` is not, and neither
|
plain text — `nav: [{ label: "shop.title" }]` is translated, `label: "Shop"` is not, and neither
|
||||||
breaks. The same holds for `config/menu.ts` branding and its `rename` overrides.
|
breaks. The same holds for `config/menu.ts` branding and its `rename` overrides.
|
||||||
- **`t()` returns raw text; the view escapes it.** Use `<%= %>` as for any other value. A message
|
- **`t()` returns raw text; the view escapes it.** Use `<%= %>` as for any other value. A message
|
||||||
that deliberately carries markup is rendered with `<%- %>` — and must never interpolate
|
that deliberately carries markup is rendered with `<%- %>` — and its `{{vars}}` must then be
|
||||||
untrusted data, since nothing escapes it there.
|
escaped at the call site, since nothing escapes them there (`pagination.ejs` is the worked example).
|
||||||
- **Dates and numbers are `Intl`'s job**, not the catalog's: `new Intl.DateTimeFormat(ctx.locale)`.
|
- **Dates and numbers are `Intl`'s job**, not the catalog's: `new Intl.DateTimeFormat(ctx.locale)`.
|
||||||
- **The core building blocks carry the locale for you** — every href they render (menu, breadcrumbs,
|
- **The core building blocks carry the locale for you** — every href they render (menu, breadcrumbs,
|
||||||
pagination, sort headers, row actions, the auth card's links) goes through `localeHref`, and their
|
pagination, sort headers, row actions, the auth card's links) goes through `localeHref`, and their
|
||||||
|
|||||||
@@ -68,3 +68,10 @@ test("pagination renders a valid empty footer and never throws on missing config
|
|||||||
assert.match(html, /<option value="50" selected>50 \/ page<\/option>/);
|
assert.match(html, /<option value="50" selected>50 \/ page<\/option>/);
|
||||||
assert.match(html, /<button class="page-btn" type="submit">Set<\/button>/);
|
assert.match(html, /<button class="page-btn" type="submit">Set<\/button>/);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("the summary message renders its markup but escapes the values interpolated into it", async () => {
|
||||||
|
// It is the one message rendered raw (<%- %>), so a value that arrives as text can't inject.
|
||||||
|
const html = await render({ summary: { from: 1, to: 12, total: '<img src=x onerror=alert(1)>' } });
|
||||||
|
assert.match(html, /<b><img src=x onerror=alert\(1\)><\/b>/);
|
||||||
|
assert.doesNotMatch(html, /<img/);
|
||||||
|
});
|
||||||
|
|||||||
@@ -18,10 +18,14 @@
|
|||||||
const next = locals.next;
|
const next = locals.next;
|
||||||
const pages = locals.pages || [];
|
const pages = locals.pages || [];
|
||||||
const eq = (a, b) => String(a ?? "") === String(b);
|
const eq = (a, b) => String(a ?? "") === String(b);
|
||||||
|
// The summary message carries markup (<b> around the total), so it renders raw — which means its
|
||||||
|
// values must be escaped here instead. Callers pass paginate() numbers today; a building block
|
||||||
|
// shouldn't depend on that.
|
||||||
|
const esc = (v) => String(v).replace(/[&<>"']/g, (c) => ({ "&": "&", "<": "<", ">": ">", '"': """, "'": "'" })[c]);
|
||||||
-%>
|
-%>
|
||||||
<footer class="pager">
|
<footer class="pager">
|
||||||
<% if (summary) { -%>
|
<% if (summary) { -%>
|
||||||
<span><%- t("pagination.summary", { from: summary.from, to: summary.to, total: summary.total }) %></span>
|
<span><%- t("pagination.summary", { from: esc(summary.from), to: esc(summary.to), total: esc(summary.total) }) %></span>
|
||||||
<% } -%>
|
<% } -%>
|
||||||
<% if (rows) { -%>
|
<% if (rows) { -%>
|
||||||
<form class="pager-rows" method="get"<% if (rows.action) { %> action="<%= localeHref(rows.action) %>"<% } %>>
|
<form class="pager-rows" method="get"<% if (rows.action) { %> action="<%= localeHref(rows.action) %>"<% } %>>
|
||||||
|
|||||||
Reference in New Issue
Block a user