mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 10:04:18 +00:00
Paul Lützner
9f4908cb9a
The compose window's send warning ("no subject?") binds an Escape handler to
`document` and unbound it only from its two answer buttons — so a window torn
down with the question unanswered left the listener behind, holding the dead
controller and its whole DOM subtree alive and swallowing Escape for whatever
came next. disconnect() already closed the PLAIN-TEXT warning, one of two
siblings, which is exactly why the other was easy to miss.
Its regression test measures Set membership of handler functions rather than a
count, and that is not fussiness: a counter read -10 here, because this codebase
unbinds defensively — both warnings remove listeners whether or not they ever
added them — so additions-minus-removals measures coding style, not leaks. It
tears the window down with its own close button, because a dock window
deliberately survives a Turbo visit and navigating away never disconnects
anything. Confirmed to fail without the fix.
connect() and disconnect() were also the only two of six form lookups in that
file with no null guard, and the indentation in disconnect showed where the
`if` had been lost. Nothing could reach it — the form is unconditional in the
template — but everything below it runs after it, so a throw there would have
skipped the listener removals and left `document.body.style.overflow` at
`hidden`, which on a phone is a page that cannot scroll.
Nine controllers had grown the same line for the CSRF token, still byte for
byte identical, which is the moment to collapse them rather than the moment
after — the server side of this had already drifted into six spellings. A
missing meta tag now says so once instead of quietly sending an empty string
and turning a layout bug into an unexplained 403. account_order keeps its own
Stimulus value: that is a different token, not a different way of reading the
same one.
Also gone: four debug console.logs, one of them printing label activity on
every use; address_autocomplete_controller, whose connect() and disconnect()
were 110 lines of commented-out TomSelect and which no template has mounted for
some time; and @rails/request.js, declared in the importmap and imported by
nothing since @stimulus-components/sortable was dropped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 38d598abb7782a98b5c830310cc023c36b4884af)
(cherry picked from commit f24fb70a5d6d0a87e5cde27f887aba4e0e4fb9fe)
(cherry picked from commit d7ba3b119ff2d86c528bd2ee79ed638693436a09)
69 lines
2.7 KiB
JavaScript
69 lines
2.7 KiB
JavaScript
/**
|
|
* The `ajax` CSRF token, for the fetch() calls that are not form submissions.
|
|
*
|
|
* Turbo-driven forms carry their own token in the body and need nothing from
|
|
* here. This is for the other half of the app: the Stimulus controllers that
|
|
* POST JSON — status buttons, bulk actions, the label menu, sidebar and pane
|
|
* preferences, sync-now — which have no form and so send the token in the
|
|
* `X-CSRF-Token` header instead. `templates/_layout/app.html.twig` stamps it
|
|
* into a `csrf-token` meta tag once per render; this reads it back.
|
|
*
|
|
* Its own module because nine controllers had grown the same line:
|
|
*
|
|
* document.querySelector('meta[name="csrf-token"]')?.content ?? ""
|
|
*
|
|
* All nine were still byte-identical, which is the moment to collapse them
|
|
* rather than the moment after — the server side of this had already drifted
|
|
* into six spellings before it was consolidated into the ChecksCsrf trait, and
|
|
* this is the same duplication seen from the browser.
|
|
*
|
|
* A tenth caller, settings/account_order_controller, takes its token as a
|
|
* Stimulus value from the template instead. That is left alone deliberately:
|
|
* it is a per-action token rather than the shared `ajax` one, so it is a
|
|
* different token, not a different way of reading the same one.
|
|
*/
|
|
|
|
/**
|
|
* The token, or an empty string when the page has no meta tag.
|
|
*
|
|
* The empty string is deliberate rather than a throw: a caller that sends
|
|
* nothing gets a clean 403 from the server, which is the correct answer and
|
|
* the same one a genuinely forged request gets. What it also used to be was
|
|
* indistinguishable from a real CSRF failure while debugging, so the missing
|
|
* tag now says so once in the console — this is a layout bug, not a user one,
|
|
* and it means every JSON POST on the page is about to fail.
|
|
*
|
|
* @param {Document} [doc]
|
|
* @returns {string}
|
|
*/
|
|
export function csrfToken(doc = document) {
|
|
const token = doc?.querySelector('meta[name="csrf-token"]')?.content;
|
|
|
|
if (!token) {
|
|
console.warn(
|
|
'[csrf] No <meta name="csrf-token"> on this page — every JSON POST from it will be refused. ' +
|
|
'The tag is rendered by templates/_layout/app.html.twig; a page that does not extend it needs its own.',
|
|
);
|
|
|
|
return "";
|
|
}
|
|
|
|
return token;
|
|
}
|
|
|
|
/**
|
|
* Headers for a JSON POST that has to prove it came from our own page.
|
|
*
|
|
* The pair travels together at every call site, so it is offered as a pair.
|
|
*
|
|
* @param {Record<string, string>} [extra] merged in, and able to override
|
|
* @returns {Record<string, string>}
|
|
*/
|
|
export function jsonCsrfHeaders(extra = {}) {
|
|
return {
|
|
"Content-Type": "application/json",
|
|
"X-CSRF-Token": csrfToken(),
|
|
...extra,
|
|
};
|
|
}
|