diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-08 13:21:02 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-08 13:21:02 +0200 |
| commit | bb8aad22c4d41dd1b89c85c8d46878a31a9530e5 (patch) | |
| tree | 65ca27637b81aa046e71c66204ff96ec138ac06f /packages/meshbay-hub/tests/test_memory_ceiling.py | |
| parent | 3f2bb22586d3e1aef765149b555ccc8e174ce7eb (diff) | |
| download | meshbay-bb8aad22c4d41dd1b89c85c8d46878a31a9530e5.tar.gz | |
fix(hub): never collect a large download in the page
`pipelinedDownload` with no writable allocates `new Array(totalChunks)` and
keeps every decrypted chunk, so whatever `_openDownloadTarget` returns null for
is held whole in RAM. That floor had no upper bound: the
`!window.showSaveFilePicker` branch returned null at any size, so on a browser
without the File System Access API a 20 GB film went to memory whenever the
streamed path did not answer. Nothing logged, nothing refused; the symptom was
the tab dying, with no error attributable to this code.
MEMORY_CEILING is 100 MB and every `return null` in that chain now goes through
a guard that throws above it. The refusal names the size, the limit and why the
streamed path declined, and lands in the transfers panel as a failed transfer
rather than in a console nobody opens.
This is a guard, not a limit on what can be downloaded: with the streamed path
primed and retried (previous commit), a file of any size still goes to disk
progressively on every browser. Two things had to change for that to be true:
- the streamed path is now tried in "ask" mode too, for a file over the ceiling
on a browser with no Save As of its own. The mode decides whether to show a
dialog; it was silently deciding whether a film could be downloaded at all;
- FilePreview had no size check whatsoever — a multi-gigabyte PDF or .csv was
fetched whole, and the text branch decoded all of it to keep 500 000
characters. It refuses above the same ceiling and offers the download.
ZIP_MAX_BYTES (512 MB) and the ceiling do not contradict: the archive limit
bounds the archive, the ceiling bounds what may be built in the page, so a
400 MB zip is allowed when there is somewhere to stream it and refused when the
only route left is memory. The build-in-memory confirmation only appears below
the ceiling now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGdheDLxGReuKHga3BtST
Diffstat (limited to 'packages/meshbay-hub/tests/test_memory_ceiling.py')
| -rw-r--r-- | packages/meshbay-hub/tests/test_memory_ceiling.py | 209 |
1 files changed, 209 insertions, 0 deletions
diff --git a/packages/meshbay-hub/tests/test_memory_ceiling.py b/packages/meshbay-hub/tests/test_memory_ceiling.py new file mode 100644 index 0000000..9966e48 --- /dev/null +++ b/packages/meshbay-hub/tests/test_memory_ceiling.py @@ -0,0 +1,209 @@ +""" +No download above the ceiling is ever collected in the page. + +`pipelinedDownload` with no `writable` allocates `new Array(totalChunks)` and +keeps every decrypted chunk, so whatever `_openDownloadTarget` returns `null` +for is a file held whole in RAM. That floor had no upper bound: the +`!window.showSaveFilePicker` branch returned `null` at any size, so on a browser +without the File System Access API a 20 GB film went to memory whenever the +service-worker path did not answer — which happens for ordinary reasons. The +symptom was the tab dying, with nothing in the source to lead back here. + +The real `_openDownloadTarget` is lifted out of `file-utils.js` **as text** and +executed against stubbed browsers, on the rule this repo already follows for the +video player: model the environment, never the code under test. A test that +transcribed the decision tree would agree with a broken version of it by +construction. + +`test_no_unguarded_memory_floor` is the one that outlives today's branches: it +reads the function and fails if a `return null` appears in it that does not go +through the guard — which is what a fourth fallback added in a hurry would look +like. +""" + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +FILE_UTILS = STATIC / "file-utils.js" + +pytestmark = pytest.mark.skipif( + shutil.which("node") is None or not FILE_UTILS.exists(), + reason="node or the SPA sources are not available") + +CEILING = 100 * 1024 * 1024 +GB = 1024 * 1024 * 1024 + + +def _lift(name, source): + """The text of one top-level declaration, from its opening line to the + column-0 brace that closes it. Nothing is re-typed into this test.""" + start = source.index(name) + end = source.index("\n}\n", start) + len("\n}\n") + return source[start:end] + + +@pytest.fixture(scope="module") +def target_fn(): + """The ceiling, its error and the real function — read, never re-typed.""" + src = FILE_UTILS.read_text() + ceiling = re.search(r"^const MEMORY_CEILING = .*?;$", src, re.M) + assert ceiling, "MEMORY_CEILING is gone from file-utils.js" + # The test's own CEILING constant must agree with the source's, or every + # boundary case below is asserting against a number nothing uses. + assert str(CEILING) in ceiling.group(0).replace(" ", "") or \ + eval(ceiling.group(0).split("=")[1].strip(" ;")) == CEILING + return "\n".join([ + ceiling.group(0), + _lift("class TooLargeForMemoryError", src), + _lift("async function _openDownloadTarget", src), + ]) + + +def _run(target_fn, tmp_path, *, size, native=False, granted=False, + streamed=False, picker=False, mode="auto"): + """Drive the real function against one browser shape.""" + script = tmp_path / "case.mjs" + script.write_text(f""" +// Stubs for everything the lifted function reaches. `formatSize` and `t` only +// build the message; the assertions are about which branch was taken. +const formatSize = (n) => `${{n}} B`; +const t = (key, vars) => key + ' ' + JSON.stringify(vars); +const platform = {{ + capabilities: {{ nativeSave: {json.dumps(native)} }}, + nativeSave: async () => ({{ name: 'n', writable: {{}} }}), + bridgeMessage: (e) => String(e), +}}; +const downloads = {{ + BLOB_LIMIT: 512 * 1024 * 1024, + // Called by the refusal to name why the streamed path declined -- absent + // from this stub, the error constructor threw TypeError and the test saw the + // wrong failure entirely. + lastStreamFailure: () => 'stubbed: no streamed target in this harness', + getMode: () => {json.dumps(mode)}, + openTarget: async () => ({json.dumps(granted)} ? {{ name: 'g', writable: {{}} }} : null), + openStreamedDownload: async () => + ({json.dumps(streamed)} ? {{ name: 's', writable: {{}} }} : null), +}}; +globalThis.window = {{}}; +if ({json.dumps(picker)}) {{ + window.showSaveFilePicker = async () => ({{ + name: 'p', createWritable: async () => ({{}}), + }}); +}} + +{target_fn} + +let outcome; +try {{ + const r = await _openDownloadTarget('film.mkv', {size}); + outcome = r === null ? {{ kind: 'memory' }} + : r === false ? {{ kind: 'cancelled' }} + : {{ kind: 'stream', name: r.name }}; +}} catch (err) {{ + outcome = {{ kind: 'refused', name: err.name, message: err.message }}; +}} +console.log(JSON.stringify(outcome)); +""") + proc = subprocess.run(["node", str(script)], capture_output=True, text=True) + assert proc.returncode == 0, proc.stderr + return json.loads(proc.stdout) + + +# ── The hole this was written for ─────────────────────────────────────────── + +def test_a_film_is_refused_rather_than_collected_in_memory(target_fn, tmp_path): + """Firefox/Safari shape: no picker, no granted folder, the worker did not + answer. This returned null — 20 GB into a tab.""" + out = _run(target_fn, tmp_path, size=20 * GB) + assert out["kind"] == "refused", out + assert out["name"] == "TooLargeForMemoryError" + + +def test_the_refusal_says_how_big_and_what_the_limit_is(target_fn, tmp_path): + out = _run(target_fn, tmp_path, size=20 * GB) + assert "download.too_large_for_memory" in out["message"] + assert str(20 * GB) in out["message"] + assert str(CEILING) in out["message"] + + +def test_the_same_browser_in_ask_mode_is_refused_too(target_fn, tmp_path): + """'ask' skips the service-worker block entirely, so it reached the + unguarded branch without even trying to stream.""" + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask") + assert out["kind"] == "refused", out + + +# ── What must keep working ────────────────────────────────────────────────── + +def test_something_small_still_uses_the_memory_floor(target_fn, tmp_path): + out = _run(target_fn, tmp_path, size=4 * 1024 * 1024) + assert out["kind"] == "memory", out + + +def test_the_boundary_is_the_ceiling_itself(target_fn, tmp_path): + assert _run(target_fn, tmp_path, size=CEILING)["kind"] == "memory" + assert _run(target_fn, tmp_path, size=CEILING + 1)["kind"] == "refused" + + +def test_a_granted_folder_streams_whatever_the_size(target_fn, tmp_path): + out = _run(target_fn, tmp_path, size=20 * GB, granted=True) + assert out == {"kind": "stream", "name": "g"} + + +def test_the_service_worker_streams_whatever_the_size(target_fn, tmp_path): + out = _run(target_fn, tmp_path, size=20 * GB, streamed=True) + assert out == {"kind": "stream", "name": "s"} + + +def test_the_desktop_app_streams_whatever_the_size(target_fn, tmp_path): + out = _run(target_fn, tmp_path, size=20 * GB, native=True) + assert out == {"kind": "stream", "name": "n"} + + +def test_a_browser_with_a_picker_is_offered_one_instead_of_being_refused( + target_fn, tmp_path): + """Chrome/Edge: the file is large, nothing streamed yet, but Save As does. + A refusal here would be this fix breaking a path that was never broken.""" + out = _run(target_fn, tmp_path, size=20 * GB, picker=True) + assert out == {"kind": "stream", "name": "p"} + + +# ── The one that outlives today's branches ────────────────────────────────── + +def test_no_unguarded_memory_floor(target_fn): + """Every `return null` in the function goes through the guard. + + A fourth fallback appended to the chain — which is exactly how the third one + got here — is caught by this even though no case above covers it. + """ + body = target_fn[target_fn.index("async function _openDownloadTarget"):] + lines = body.splitlines() + # The guard's own `return null` is the one legitimate instance, so cut its + # definition out before looking. Comments go too — the branch that used to + # be the bug is now described in one, and a test that reads prose is the + # mistake already recorded in CLAUDE.md for the packaged systemd unit. + start = next(n for n, l in enumerate(lines) if "const _memoryFloor" in l) + end = next(n for n in range(start, len(lines)) if lines[n].strip() == "};") + rest = lines[:start] + lines[end + 1:] + code = [re.sub(r"//.*$", "", l) for l in rest] + bare = [l.strip() for l in code if re.search(r"\breturn null\b", l)] + assert bare == [], ( + "an unguarded in-memory fallback was added to _openDownloadTarget; " + "return _memoryFloor() instead: " + "; ".join(bare)) + + +def test_the_guard_is_what_the_preview_uses_too(target_fn): + """`FilePreview` decrypts a whole entry with no writable at all, so it needs + the same ceiling — and must import it rather than keep a second number.""" + files_app = (STATIC / "files-app.js").read_text() + assert "MEMORY_CEILING" in files_app + assert re.search(r"entry\.size\s*>\s*MEMORY_CEILING", files_app), ( + "the preview modal must refuse an oversized entry before fetching it") + assert not re.search(r"100\s*\*\s*1024\s*\*\s*1024", files_app), ( + "the ceiling is defined once, in file-utils.js") |