Merge pull request #127 from LINUXexpert-org/blob-download-compressed-length
Send the length of the bytes we are actually sending
This commit is contained in:
@@ -21,6 +21,8 @@ works the same way — and dropped where 0.15 was the whole subject. Support for
|
|||||||
0.15 was removed on 2026-08-26; the last release that runs on it is tagged
|
0.15 was removed on 2026-08-26; the last release that runs on it is tagged
|
||||||
[`stalwart-0.15-support`](https://github.com/LINUXexpert-org/ihasmail/releases/tag/stalwart-0.15-support).
|
[`stalwart-0.15-support`](https://github.com/LINUXexpert-org/ihasmail/releases/tag/stalwart-0.15-support).
|
||||||
|
|
||||||
|
- **A compressing hop in front of Stalwart truncated every blob download, and nothing said so.** Node decompresses a gzip response before the code ever sees the body, but leaves the `content-length` header describing the *compressed* bytes. The blob proxy copied that header onto the longer body it forwarded, so the browser stopped reading exactly that many bytes in and called the download complete. Reported on [#76](https://github.com/LINUXexpert-org/ihasmail/issues/76) against a Coolify deployment, where Traefik's compress middleware only engages above 1 KiB: filter rules one and two were fine and the third pushed the script past the threshold, after which it came back cut off mid-rule — 384 bytes of a 1.3 KB script. The size threshold is what made it look like a race. This is the *second* cause behind that issue, and the first fix did not touch it: a truncated script is neither unknown nor empty, so the "refuse to save from a baseline we could not read" guard never fired — the script parsed, just with rules missing, and the next save wrote the short version back over the real one. Every blob download shared the fault, not just Sieve: message source, vCards, signature HTML, attachments being forwarded, and the `settings.json` sync. Settings degraded honestly by luck rather than design — a truncated file fails `JSON.parse`, which is caught and leaves the local cache in charge — so it stopped syncing between devices instead of being overwritten. The proxy now asks upstream for `identity` and, for a hop that compresses anyway, forwards no length at all rather than one describing different bytes. The image proxy is unaffected: it uses `node:http` directly, sends no `accept-encoding`, and never decompresses. The save path no longer trusts the transport either: a script is now checked for completeness against the shape the generator emits — every `# rule:` comment parses, every enabled rule has an `if` and a closed body below it, every block ends with a blank line — and saving refuses on anything short, as does the rule editor, which reports the script as unreadable rather than showing the rules that happened to parse. The check is structural rather than a re-serialize-and-compare, so a script written by an older version with a different serializer is still editable; refusing over a changed byte would be the worse bug. It catches a cut at every offset except the end of a complete rule block, which is a legitimately shorter script and indistinguishable from one in the bytes alone — that residual is what the proxy fix covers.
|
||||||
|
|
||||||
- **Delete all spam destroys, and does not pass through Deleted Items** — this is the point of the feature and the thing worth checking on a real server, since a folder that empties into another folder has solved nothing. `Email/set destroy`, walked a page at a time so it survives `maxObjectsInSet` the way emptying Deleted Items already had to. **Confirmed live on 0.16.19 (2026-08-26)**: Junk Mail emptied and Deleted Items stayed empty afterwards. There is no undo, which is why all three entry points share one dialog that says so. Only Deleted Items and Junk Mail can be emptied this way, enforced in the store rather than only hidden in the menus.
|
- **Delete all spam destroys, and does not pass through Deleted Items** — this is the point of the feature and the thing worth checking on a real server, since a folder that empties into another folder has solved nothing. `Email/set destroy`, walked a page at a time so it survives `maxObjectsInSet` the way emptying Deleted Items already had to. **Confirmed live on 0.16.19 (2026-08-26)**: Junk Mail emptied and Deleted Items stayed empty afterwards. There is no undo, which is why all three entry points share one dialog that says so. Only Deleted Items and Junk Mail can be emptied this way, enforced in the store rather than only hidden in the menus.
|
||||||
- **Sharing a mail folder is accepted and does nothing.** `Mailbox/set` with a `shareWith` map is applied, `Mailbox/get` reads it back, and the folder never appears for the account it was shared with — **confirmed live on 0.16.19 (2026-08-27)** with a folder shared read-only to another account on the same server, which never saw it. Stalwart's own sharing documentation lists calendars, address books and file storage; mail folders are not among them. Nothing reports a failure at any point, which is the whole problem: the share is stored, so a client that trusts what it reads back shows it as live for ever. The entry point is withdrawn. A folder that is *already* shared still offers **Stop sharing**, because a share nobody can see is exactly the one you want to be able to clear, and there is no other way to. File sharing is unaffected and works end to end.
|
- **Sharing a mail folder is accepted and does nothing.** `Mailbox/set` with a `shareWith` map is applied, `Mailbox/get` reads it back, and the folder never appears for the account it was shared with — **confirmed live on 0.16.19 (2026-08-27)** with a folder shared read-only to another account on the same server, which never saw it. Stalwart's own sharing documentation lists calendars, address books and file storage; mail folders are not among them. Nothing reports a failure at any point, which is the whole problem: the share is stored, so a client that trusts what it reads back shows it as live for ever. The entry point is withdrawn. A folder that is *already* shared still offers **Stop sharing**, because a share nobody can see is exactly the one you want to be able to clear, and there is no other way to. File sharing is unaffected and works end to end.
|
||||||
- **Address book sharing works, and was briefly withdrawn by mistake.** It was taken out alongside mail folders on 2026-08-27 on a report that it behaved the same way; the report was mistaken and the feature was put back the same day. Nothing was ever shown to be wrong with it, and Stalwart documents address books as shareable. Recorded because the withdrawal is in the history and would otherwise read as a finding. Shared books now appear in the Contacts pane under "Shared with me" rather than behind an account switch, and their contacts are offered when addressing a message.
|
- **Address book sharing works, and was briefly withdrawn by mistake.** It was taken out alongside mail folders on 2026-08-27 on a report that it behaved the same way; the report was mistaken and the feature was put back the same day. Nothing was ever shown to be wrong with it, and Stalwart documents address books as shareable. Recorded because the withdrawal is in the history and would otherwise read as a finding. Shared books now appear in the Contacts pane under "Shared with me" rather than behind an account switch, and their contacts are offered when addressing a message.
|
||||||
|
|||||||
@@ -35,3 +35,56 @@ test("image proxy refuses private targets", async () => {
|
|||||||
const res = await app.request("/api/image?url=http://127.0.0.1/x");
|
const res = await app.request("/api/image?url=http://127.0.0.1/x");
|
||||||
assert.equal(res.status, 401);
|
assert.equal(res.status, 401);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("a compressed upstream blob is not forwarded with the compressed length", async () => {
|
||||||
|
const { forwardedContentLength } = await import("./app.js");
|
||||||
|
// gzip: the body we forward has already been decompressed, so the length on
|
||||||
|
// the wire describes different bytes and must not be copied (issue #76).
|
||||||
|
const gz = new Headers({ "content-encoding": "gzip", "content-length": "384" });
|
||||||
|
assert.equal(forwardedContentLength(gz), null);
|
||||||
|
// identity, spelled out or absent: the length describes the body we send.
|
||||||
|
assert.equal(forwardedContentLength(new Headers({ "content-encoding": "identity", "content-length": "1157" })), "1157");
|
||||||
|
assert.equal(forwardedContentLength(new Headers({ "content-length": "1157" })), "1157");
|
||||||
|
assert.equal(forwardedContentLength(new Headers({ "content-encoding": "BR", "content-length": "384" })), null);
|
||||||
|
// Nothing to forward is not an error.
|
||||||
|
assert.equal(forwardedContentLength(new Headers()), null);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a Sieve script larger than a compressing hop's threshold survives the proxy", async () => {
|
||||||
|
const http = await import("node:http");
|
||||||
|
const zlib = await import("node:zlib");
|
||||||
|
const { forwardedContentLength } = await import("./app.js");
|
||||||
|
|
||||||
|
const script =
|
||||||
|
"# ihasmail filters v1 - edit with care; rules are stored in the `# rule:` comments\nrequire [\"fileinto\"];\n\n" +
|
||||||
|
["a", "b", "c"]
|
||||||
|
.map(
|
||||||
|
(k) =>
|
||||||
|
`# rule:{"id":"r${k}","name":"From ${k}@example.com","enabled":true,"join":"allof","tests":[{"type":"header","header":"from","op":"contains","value":"${k}@example.com"}],"actions":[{"type":"fileinto","mailbox":"INBOX/${k}"}]}\n` +
|
||||||
|
`if header :contains "from" "${k}@example.com"\n{\n fileinto "INBOX/${k}";\n}\n\n`,
|
||||||
|
)
|
||||||
|
.join("");
|
||||||
|
const gz = zlib.gzipSync(Buffer.from(script));
|
||||||
|
assert.ok(gz.length < Buffer.byteLength(script), "the script has to compress for this test to mean anything");
|
||||||
|
|
||||||
|
// A hop that compresses regardless of what we asked for.
|
||||||
|
const origin = http.createServer((_req, res) => {
|
||||||
|
res.writeHead(200, { "content-type": "application/sieve", "content-encoding": "gzip", "content-length": String(gz.length) });
|
||||||
|
res.end(gz);
|
||||||
|
});
|
||||||
|
await new Promise<void>((r) => origin.listen(0, () => r()));
|
||||||
|
const port = (origin.address() as { port: number }).port;
|
||||||
|
|
||||||
|
try {
|
||||||
|
const up = await fetch(`http://127.0.0.1:${port}/`);
|
||||||
|
// What the blob route forwards.
|
||||||
|
const headers = new Headers({ "content-type": "application/sieve; charset=utf-8" });
|
||||||
|
const cl = forwardedContentLength(up.headers);
|
||||||
|
if (cl) headers.set("Content-Length", cl);
|
||||||
|
const out = new Response(await up.arrayBuffer(), { status: 200, headers });
|
||||||
|
assert.equal(out.headers.get("content-length"), null);
|
||||||
|
assert.equal(await out.text(), script);
|
||||||
|
} finally {
|
||||||
|
origin.close();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
+31
-2
@@ -519,14 +519,17 @@ export function createApp(): Hono<Env> {
|
|||||||
const upstream = await getUpstreamSession(session.id, session.authorization);
|
const upstream = await getUpstreamSession(session.id, session.authorization);
|
||||||
const url = absoluteUpstream(expandTemplate(upstream.downloadUrl, { accountId, blobId, name, type: accept }));
|
const url = absoluteUpstream(expandTemplate(upstream.downloadUrl, { accountId, blobId, name, type: accept }));
|
||||||
const res = await fetch(url, {
|
const res = await fetch(url, {
|
||||||
headers: { authorization: session.authorization },
|
// Ask for the bytes as they are. undici would otherwise negotiate gzip
|
||||||
|
// on our behalf and hand back a decompressed body whose content-length
|
||||||
|
// header still describes the compressed one -- see forwardedContentLength.
|
||||||
|
headers: { authorization: session.authorization, "accept-encoding": "identity" },
|
||||||
signal: AbortSignal.timeout(Math.max(config.upstreamTimeout, 5 * 60_000)),
|
signal: AbortSignal.timeout(Math.max(config.upstreamTimeout, 5 * 60_000)),
|
||||||
});
|
});
|
||||||
if (!res.ok) return c.json({ error: "not_found" }, res.status === 404 ? 404 : 502);
|
if (!res.ok) return c.json({ error: "not_found" }, res.status === 404 ? 404 : 502);
|
||||||
const headers = new Headers();
|
const headers = new Headers();
|
||||||
const type = sanitizeContentType(res.headers.get("content-type") ?? accept);
|
const type = sanitizeContentType(res.headers.get("content-type") ?? accept);
|
||||||
headers.set("Content-Type", type);
|
headers.set("Content-Type", type);
|
||||||
const cl = res.headers.get("content-length");
|
const cl = forwardedContentLength(res.headers);
|
||||||
if (cl) headers.set("Content-Length", cl);
|
if (cl) headers.set("Content-Length", cl);
|
||||||
const safeInline = inline && isInlineSafe(type);
|
const safeInline = inline && isInlineSafe(type);
|
||||||
headers.set(
|
headers.set(
|
||||||
@@ -651,6 +654,32 @@ function passthrough(res: Response): Response {
|
|||||||
return new Response(res.body, { status: res.status, headers });
|
return new Response(res.body, { status: res.status, headers });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The upstream content-length, but only when it describes the bytes we are
|
||||||
|
* about to forward.
|
||||||
|
*
|
||||||
|
* A compressed response is decompressed for us before we ever see the body --
|
||||||
|
* undici does it transparently -- while the content-length header is left
|
||||||
|
* describing the *compressed* length. Copying it onto the longer body we then
|
||||||
|
* send makes the browser stop reading exactly that many bytes in and call the
|
||||||
|
* download complete, so the file arrives silently truncated.
|
||||||
|
*
|
||||||
|
* That is the second half of issue #76. A hop in front of Stalwart compressed
|
||||||
|
* responses over 1 KiB, so a Sieve script stayed intact until the third rule
|
||||||
|
* pushed it past the threshold and it came back cut off mid-rule. Nothing
|
||||||
|
* reported an error: the script parsed, just with rules missing, and saving
|
||||||
|
* wrote that shortened version back over the real one.
|
||||||
|
*
|
||||||
|
* We ask for `identity` above so the usual case still carries a length the
|
||||||
|
* browser can show progress against; this is the guard for a hop that
|
||||||
|
* compresses anyway.
|
||||||
|
*/
|
||||||
|
export function forwardedContentLength(headers: Headers): string | null {
|
||||||
|
const encoding = headers.get("content-encoding")?.trim().toLowerCase();
|
||||||
|
if (encoding && encoding !== "identity") return null;
|
||||||
|
return headers.get("content-length");
|
||||||
|
}
|
||||||
|
|
||||||
function sanitizeContentType(ct: string): string {
|
function sanitizeContentType(ct: string): string {
|
||||||
const lower = ct.split(";")[0]!.trim().toLowerCase();
|
const lower = ct.split(";")[0]!.trim().toLowerCase();
|
||||||
// Never let the browser render HTML/SVG/XML/JS served from the blob endpoint.
|
// Never let the browser render HTML/SVG/XML/JS served from the blob endpoint.
|
||||||
|
|||||||
@@ -174,6 +174,100 @@ export function rulesToSieve(rules: SieveRule[]): string {
|
|||||||
return lines.join("\n");
|
return lines.join("\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Why this script must not be rewritten from the rules parsed out of it, or
|
||||||
|
* null when rewriting it is safe.
|
||||||
|
*
|
||||||
|
* Saving replaces the whole script with a fresh serialization of the rules read
|
||||||
|
* out of it, so whatever was not read is deleted. `sieveToRules` cannot raise
|
||||||
|
* the alarm by itself: it skips what it does not recognise, so a script cut off
|
||||||
|
* partway through parses cleanly into a shorter list and looks exactly like one
|
||||||
|
* that genuinely has fewer rules. That is the shape of the loss in #76 -- a
|
||||||
|
* truncated download, a plausible parse, and a save that wrote the short
|
||||||
|
* version back over the real one. The transport fault behind it is fixed in the
|
||||||
|
* blob proxy; this is the check that makes the save path refuse regardless of
|
||||||
|
* how the content came to be short.
|
||||||
|
*
|
||||||
|
* The tests are structural rather than an equality check against
|
||||||
|
* `rulesToSieve(sieveToRules(content))`. A script written by an older version
|
||||||
|
* whose serializer differed in some detail is intact, and refusing to let
|
||||||
|
* anyone edit their rules over a changed byte would be the worse bug.
|
||||||
|
*/
|
||||||
|
export function scriptDamage(content: string): string | null {
|
||||||
|
// Not one of ours: genuinely empty, or hand-written. Both mean something
|
||||||
|
// else and are answered elsewhere.
|
||||||
|
if (!content.includes("# rule:") && !content.includes(SCRIPT_HEADER)) {
|
||||||
|
// Unless it is one of ours cut off inside its own first line, which reads
|
||||||
|
// as a very short hand-written script -- and that reading is the one that
|
||||||
|
// offers to replace it.
|
||||||
|
const head = content.replace(/\n+$/, "");
|
||||||
|
if (head !== "" && SCRIPT_HEADER.startsWith(head)) return "breaks off inside its first line";
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
// Every generated script ends with a newline, so a body that stops mid-line
|
||||||
|
// stopped early. The rest of the walk covers the cuts that land on one.
|
||||||
|
if (!content.endsWith("\n")) return "stops in the middle of a line";
|
||||||
|
|
||||||
|
const lines = content.replace(/\r\n/g, "\n").split("\n");
|
||||||
|
let i = 0;
|
||||||
|
let hadRequire = false;
|
||||||
|
if (lines[0] === SCRIPT_HEADER) {
|
||||||
|
i = 1;
|
||||||
|
hadRequire = lines[i]?.startsWith("require ") ?? false;
|
||||||
|
if (hadRequire) i++;
|
||||||
|
if (lines[i] !== "") return "breaks off in its opening lines";
|
||||||
|
i++;
|
||||||
|
} else {
|
||||||
|
// Header edited away but the rule comments kept. Still ours to walk.
|
||||||
|
i = lines.findIndex((l) => l.startsWith("# rule:"));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Walk the shape rulesToSieve emits, one rule block at a time. Deliberately
|
||||||
|
// structural: the condition and action lines are read only for their
|
||||||
|
// presence, so a serializer that words them differently is still intact.
|
||||||
|
let seen = 0;
|
||||||
|
const cut = (r: SieveRule, what: string) => `has a rule in it (“${r.name}”) ${what}`;
|
||||||
|
while (i < lines.length) {
|
||||||
|
const line = lines[i]!;
|
||||||
|
if (!line.startsWith("# rule:")) return "has a stray line where a rule should start";
|
||||||
|
let rule: SieveRule | null = null;
|
||||||
|
try {
|
||||||
|
const parsed = JSON.parse(line.slice(7)) as SieveRule;
|
||||||
|
if (parsed && typeof parsed === "object" && Array.isArray(parsed.tests) && Array.isArray(parsed.actions)) rule = parsed;
|
||||||
|
} catch {
|
||||||
|
/* reported below */
|
||||||
|
}
|
||||||
|
if (!rule) return "has a rule in it that breaks off unfinished";
|
||||||
|
seen++;
|
||||||
|
i++;
|
||||||
|
// `!r.enabled` is how rulesToSieve chooses the branch, so match it exactly
|
||||||
|
// rather than testing for `=== false`.
|
||||||
|
if (rule.enabled) {
|
||||||
|
if (!lines[i]?.startsWith("if ")) return cut(rule, "with nothing below it");
|
||||||
|
i++;
|
||||||
|
if (lines[i] !== "{") return cut(rule, "whose body never opens");
|
||||||
|
i++;
|
||||||
|
while (i < lines.length && lines[i] !== "}") {
|
||||||
|
if (lines[i] === "") return cut(rule, "whose body breaks off");
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
if (i >= lines.length) return cut(rule, "whose body never closes");
|
||||||
|
i++;
|
||||||
|
} else {
|
||||||
|
if (!lines[i]?.startsWith("# (disabled) ")) return cut(rule, "with nothing below it");
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
// Each block is followed by a blank line, the last one included: it is the
|
||||||
|
// empty final element left by the trailing newline.
|
||||||
|
if (lines[i] !== "") return cut(rule, "that runs into what follows it");
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
// A require line is written only for rules that need it, so one standing over
|
||||||
|
// no rules at all means the rules it was written for are gone.
|
||||||
|
if (hadRequire && seen === 0) return "breaks off before the first rule";
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
/** Returns rules if the script was generated by ihasmail, else null (raw script). */
|
/** Returns rules if the script was generated by ihasmail, else null (raw script). */
|
||||||
export function sieveToRules(script: string): SieveRule[] | null {
|
export function sieveToRules(script: string): SieveRule[] | null {
|
||||||
if (!script.includes("# rule:")) return script.trim() === "" || script.includes(SCRIPT_HEADER) ? [] : null;
|
if (!script.includes("# rule:")) return script.trim() === "" || script.includes(SCRIPT_HEADER) ? [] : null;
|
||||||
|
|||||||
@@ -83,3 +83,91 @@ describe("reloading", () => {
|
|||||||
expect(useSieve.getState().rules().rules).toHaveLength(3);
|
expect(useSieve.getState().rules().rules).toHaveLength(3);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Issue #76, second round. The transport fault is fixed in the blob proxy, but
|
||||||
|
* the save path had no answer for a script that arrives *partly* read: it is
|
||||||
|
* neither unknown nor empty, so the guards above all pass it through. It parses
|
||||||
|
* into a shorter rule list that looks exactly like a script with fewer rules,
|
||||||
|
* and saving writes that shorter version back over the real one.
|
||||||
|
*
|
||||||
|
* These pin the third state: read, but not all of it.
|
||||||
|
*/
|
||||||
|
describe("a script that was only partly read", () => {
|
||||||
|
/** Cut at 384 bytes, the way a compressing hop cut the reporter's script. */
|
||||||
|
const truncate = (content: string, at: number) => content.slice(0, at);
|
||||||
|
const full = rulesToSieve(threeRules);
|
||||||
|
|
||||||
|
it("reports its rules as unknown rather than handing back the ones that parsed", () => {
|
||||||
|
useSieve.setState({ contents: { s1: truncate(full, 384) } });
|
||||||
|
const { rules, loaded, damage } = useSieve.getState().rules();
|
||||||
|
expect(rules).toBeNull();
|
||||||
|
expect(loaded).toBe(true);
|
||||||
|
expect(damage).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("refuses to save over the part it never saw", async () => {
|
||||||
|
useSieve.setState({ contents: { s1: truncate(full, 384) } });
|
||||||
|
await expect(useSieve.getState().saveRules([newRule({ name: "New" })])).rejects.toThrow(/overwrite the rest of it/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("catches a cut at every offset through the script, not just a lucky one", () => {
|
||||||
|
// The offsets that cannot be caught are the ends of complete rule blocks:
|
||||||
|
// each is a valid shorter script and nothing in the bytes says otherwise.
|
||||||
|
// That is the residual the proxy fix covers and this check cannot.
|
||||||
|
const safe = new Set<number>();
|
||||||
|
for (let n = 0; n <= threeRules.length; n++) safe.add(rulesToSieve(threeRules.slice(0, n)).length);
|
||||||
|
let missed = 0;
|
||||||
|
for (let at = 1; at < full.length; at++) {
|
||||||
|
useSieve.setState({ contents: { s1: truncate(full, at) } });
|
||||||
|
const { damage } = useSieve.getState().rules();
|
||||||
|
if (!damage && !safe.has(at)) missed++;
|
||||||
|
}
|
||||||
|
expect(missed).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves an intact script alone at every length it can legitimately have", () => {
|
||||||
|
for (let n = 0; n <= threeRules.length; n++) {
|
||||||
|
useSieve.setState({ contents: { s1: rulesToSieve(threeRules.slice(0, n)) } });
|
||||||
|
const { rules, damage } = useSieve.getState().rules();
|
||||||
|
expect(damage).toBeNull();
|
||||||
|
expect(rules).toHaveLength(n);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves the shapes a rule can take alone — disabled, many actions, extensions", () => {
|
||||||
|
// A false positive here costs someone the use of the rules editor, so the
|
||||||
|
// walk has to pass everything rulesToSieve can legitimately produce.
|
||||||
|
const varied = [
|
||||||
|
newRule({ name: "Disabled", enabled: false }),
|
||||||
|
newRule({ name: "Many actions", actions: [{ type: "fileinto", mailbox: "A" }, { type: "markread" }, { type: "flag" }, { type: "stop" }] }),
|
||||||
|
newRule({ name: "Two tests", join: "anyof", tests: [{ type: "body", op: "contains", value: "x" }, { type: "size", op: "over", value: 1024 }] }),
|
||||||
|
newRule({ name: "No actions at all", actions: [] }),
|
||||||
|
newRule({ name: "Quotes \" and \\ backslash" }),
|
||||||
|
];
|
||||||
|
useSieve.setState({ contents: { s1: rulesToSieve(varied) } });
|
||||||
|
const { rules, damage } = useSieve.getState().rules();
|
||||||
|
expect(damage).toBeNull();
|
||||||
|
expect(rules).toHaveLength(varied.length);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("catches a cut at every offset through that script too", () => {
|
||||||
|
const varied = [newRule({ name: "Disabled", enabled: false }), newRule({ name: "Live" }), newRule({ name: "Also off", enabled: false })];
|
||||||
|
const full = rulesToSieve(varied);
|
||||||
|
const safe = new Set<number>();
|
||||||
|
for (let n = 0; n <= varied.length; n++) safe.add(rulesToSieve(varied.slice(0, n)).length);
|
||||||
|
let missed = 0;
|
||||||
|
for (let at = 1; at < full.length; at++) {
|
||||||
|
useSieve.setState({ contents: { s1: full.slice(0, at) } });
|
||||||
|
if (!useSieve.getState().rules().damage && !safe.has(at)) missed++;
|
||||||
|
}
|
||||||
|
expect(missed).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not call a hand-written script damaged", () => {
|
||||||
|
useSieve.setState({ contents: { s1: 'require ["fileinto"];\nif header :contains "from" "x" { fileinto "X"; }' } });
|
||||||
|
const { rules, damage } = useSieve.getState().rules();
|
||||||
|
expect(damage).toBeNull();
|
||||||
|
expect(rules).toBeNull(); // hand-written, which is a different refusal
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
+22
-7
@@ -1,7 +1,7 @@
|
|||||||
import { create } from "zustand";
|
import { create } from "zustand";
|
||||||
import { CAP, client, setErrorMessage } from "@/jmap/client";
|
import { CAP, client, setErrorMessage } from "@/jmap/client";
|
||||||
import type { GetResponse, Id, SetResponse, SieveScript } from "@/jmap/types";
|
import type { GetResponse, Id, SetResponse, SieveScript } from "@/jmap/types";
|
||||||
import { rulesToSieve, sieveToRules, type SieveRule } from "@/lib/sieve";
|
import { rulesToSieve, scriptDamage, sieveToRules, type SieveRule } from "@/lib/sieve";
|
||||||
import { useSession } from "./session";
|
import { useSession } from "./session";
|
||||||
|
|
||||||
export const IHASMAIL_SCRIPT = "ihasmail";
|
export const IHASMAIL_SCRIPT = "ihasmail";
|
||||||
@@ -19,7 +19,7 @@ interface SieveState {
|
|||||||
getContent(id: Id): Promise<string>;
|
getContent(id: Id): Promise<string>;
|
||||||
/** Rules derived from the "ihasmail" script (null = the active script is hand-written). */
|
/** Rules derived from the "ihasmail" script (null = the active script is hand-written). */
|
||||||
/** `loaded` distinguishes "this script is hand-written" from "we could not read it". */
|
/** `loaded` distinguishes "this script is hand-written" from "we could not read it". */
|
||||||
rules(): { script: SieveScript | null; rules: SieveRule[] | null; content: string; loaded: boolean };
|
rules(): { script: SieveScript | null; rules: SieveRule[] | null; content: string; loaded: boolean; damage: string | null };
|
||||||
saveRules(rules: SieveRule[]): Promise<void>;
|
saveRules(rules: SieveRule[]): Promise<void>;
|
||||||
saveScript(id: Id | null, name: string, content: string, activate: boolean): Promise<Id>;
|
saveScript(id: Id | null, name: string, content: string, activate: boolean): Promise<Id>;
|
||||||
activate(id: Id | null): Promise<void>;
|
activate(id: Id | null): Promise<void>;
|
||||||
@@ -91,14 +91,19 @@ export const useSieve = create<SieveState>((set, get) => ({
|
|||||||
rules() {
|
rules() {
|
||||||
const { scripts, contents } = get();
|
const { scripts, contents } = get();
|
||||||
const script = scripts.find((s) => s.name === IHASMAIL_SCRIPT) ?? scripts.find((s) => s.isActive) ?? null;
|
const script = scripts.find((s) => s.name === IHASMAIL_SCRIPT) ?? scripts.find((s) => s.isActive) ?? null;
|
||||||
if (!script) return { script: null, rules: [], content: "", loaded: true };
|
if (!script) return { script: null, rules: [], content: "", loaded: true, damage: null };
|
||||||
const content = contents[script.id];
|
const content = contents[script.id];
|
||||||
// Not loaded, or the fetch failed. `null` means "cannot say", which every
|
// Not loaded, or the fetch failed. `null` means "cannot say", which every
|
||||||
// caller already treats as "do not edit this script" -- as opposed to `[]`,
|
// caller already treats as "do not edit this script" -- as opposed to `[]`,
|
||||||
// which means "this script genuinely has no rules" and invites a save that
|
// which means "this script genuinely has no rules" and invites a save that
|
||||||
// would overwrite whatever is really in it.
|
// would overwrite whatever is really in it.
|
||||||
if (content === undefined) return { script, rules: null, content: "", loaded: false };
|
if (content === undefined) return { script, rules: null, content: "", loaded: false, damage: null };
|
||||||
return { script, rules: sieveToRules(content), content, loaded: true };
|
// Read, but not all of it. Showing the rules that did parse would be the
|
||||||
|
// most dangerous thing available: a short list that looks complete, over a
|
||||||
|
// script that is not. Say "cannot say" here too.
|
||||||
|
const damage = scriptDamage(content);
|
||||||
|
if (damage) return { script, rules: null, content, loaded: true, damage };
|
||||||
|
return { script, rules: sieveToRules(content), content, loaded: true, damage: null };
|
||||||
},
|
},
|
||||||
|
|
||||||
async saveRules(rules) {
|
async saveRules(rules) {
|
||||||
@@ -106,8 +111,18 @@ export const useSieve = create<SieveState>((set, get) => ({
|
|||||||
// The last line of defence. Writing rules replaces the whole script, so
|
// The last line of defence. Writing rules replaces the whole script, so
|
||||||
// doing it from a baseline we never managed to read deletes whatever was
|
// doing it from a baseline we never managed to read deletes whatever was
|
||||||
// there. Refusing is recoverable; overwriting is not.
|
// there. Refusing is recoverable; overwriting is not.
|
||||||
if (existing && get().contents[existing.id] === undefined) {
|
if (existing) {
|
||||||
throw new Error("Your filter script could not be read, so saving would overwrite it. Reload and try again.");
|
const content = get().contents[existing.id];
|
||||||
|
if (content === undefined) {
|
||||||
|
throw new Error("Your filter script could not be read, so saving would overwrite it. Reload and try again.");
|
||||||
|
}
|
||||||
|
// Read in full is a separate question from read at all, and the answer
|
||||||
|
// that cost rules in #76 was "partly". A baseline missing its tail writes
|
||||||
|
// out just as confidently as one missing entirely.
|
||||||
|
const damage = scriptDamage(content);
|
||||||
|
if (damage) {
|
||||||
|
throw new Error(`Your filter script ${damage}, so saving would overwrite the rest of it. Reload and try again.`);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
await get().saveScript(existing?.id ?? null, IHASMAIL_SCRIPT, rulesToSieve(rules), true);
|
await get().saveScript(existing?.id ?? null, IHASMAIL_SCRIPT, rulesToSieve(rules), true);
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -33,17 +33,19 @@ export function FilterFromMessageDialog({ email, mailboxId, onClose }: { email:
|
|||||||
}
|
}
|
||||||
if (!ready) return <Dialog open onClose={onClose} title="Create filter" size="sm"><Spinner /></Dialog>;
|
if (!ready) return <Dialog open onClose={onClose} title="Create filter" size="sm"><Spinner /></Dialog>;
|
||||||
|
|
||||||
const { rules, loaded } = sieve.rules();
|
const { rules, loaded, damage } = sieve.rules();
|
||||||
if (rules === null) {
|
if (rules === null) {
|
||||||
return (
|
return (
|
||||||
<Dialog open onClose={onClose} title="Create filter" size="sm" footer={<button className="btn" onClick={onClose}>Close</button>}>
|
<Dialog open onClose={onClose} title="Create filter" size="sm" footer={<button className="btn" onClick={onClose}>Close</button>}>
|
||||||
{/*
|
{/*
|
||||||
Two different situations, and telling them apart matters: one is
|
Three different situations, and telling them apart matters: one is
|
||||||
permanent and one is a reload away. Saying "written by hand" when the
|
permanent and two are a reload away. Saying "written by hand" when the
|
||||||
script merely failed to fetch sends someone looking for a problem
|
script merely failed to fetch -- or arrived in part -- sends someone
|
||||||
they do not have.
|
looking for a problem they do not have.
|
||||||
*/}
|
*/}
|
||||||
{loaded ? (
|
{damage ? (
|
||||||
|
<p>Your filter script {damage}, so only part of it arrived. Adding a rule would write that part back over the whole thing. Reload the page and try again.</p>
|
||||||
|
) : loaded ? (
|
||||||
<p>Your active Sieve script was written by hand, so rules can't be added automatically. Open <b>Settings → Filters & rules</b> to edit the script or switch to managed rules.</p>
|
<p>Your active Sieve script was written by hand, so rules can't be added automatically. Open <b>Settings → Filters & rules</b> to edit the script or switch to managed rules.</p>
|
||||||
) : (
|
) : (
|
||||||
<p>Your filter script couldn't be read just now, so adding a rule would risk overwriting it. Reload the page and try again.</p>
|
<p>Your filter script couldn't be read just now, so adding a rule would risk overwriting it. Reload the page and try again.</p>
|
||||||
|
|||||||
@@ -45,7 +45,7 @@ const RULE_MIME = "application/x-ihasmail-sieve-rule";
|
|||||||
|
|
||||||
function RulesEditor() {
|
function RulesEditor() {
|
||||||
const sieve = useSieve();
|
const sieve = useSieve();
|
||||||
const { script, rules, content } = sieve.rules();
|
const { script, rules, content, damage } = sieve.rules();
|
||||||
const [local, setLocal] = useState<SieveRule[] | null>(null);
|
const [local, setLocal] = useState<SieveRule[] | null>(null);
|
||||||
const [editing, setEditing] = useState<SieveRule | null>(null);
|
const [editing, setEditing] = useState<SieveRule | null>(null);
|
||||||
const [saving, setSaving] = useState(false);
|
const [saving, setSaving] = useState(false);
|
||||||
@@ -80,6 +80,19 @@ function RulesEditor() {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Before the hand-written branch: a script that arrived in part is not a
|
||||||
|
// script someone chose to write themselves, and the way out of it is a reload
|
||||||
|
// rather than the "start with rules" button below, which would write over it.
|
||||||
|
if (damage) {
|
||||||
|
return (
|
||||||
|
<div className="warn-box">
|
||||||
|
<div className="row gap-8" style={{ marginBottom: 8 }}><AlertTriangle size={18} /> <b>Only part of your filter script arrived.</b></div>
|
||||||
|
<p style={{ margin: "0 0 8px" }}>It {damage}, so the rules in it can't be shown or edited — saving what did arrive would write it back over the rest. Reload the page to try again. Your rules are still on the server; nothing here has changed them.</p>
|
||||||
|
<button className="btn" onClick={() => window.location.reload()}>Reload</button>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
if (rules === null) {
|
if (rules === null) {
|
||||||
return (
|
return (
|
||||||
<div className="warn-box">
|
<div className="warn-box">
|
||||||
|
|||||||
Reference in New Issue
Block a user