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_zip_size_limit.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_zip_size_limit.py')
| -rw-r--r-- | packages/meshbay-hub/tests/test_zip_size_limit.py | 59 |
1 files changed, 53 insertions, 6 deletions
diff --git a/packages/meshbay-hub/tests/test_zip_size_limit.py b/packages/meshbay-hub/tests/test_zip_size_limit.py index 203c10c..44ad59e 100644 --- a/packages/meshbay-hub/tests/test_zip_size_limit.py +++ b/packages/meshbay-hub/tests/test_zip_size_limit.py @@ -6,11 +6,16 @@ folder as a zip" button — Files' single folder, Files' multi-folder selection, and the Photos album button (docs/photos.md §3) — so the limit is checked once, there, and holds for all of them. -Two things are worth pinning. That an oversized folder is refused *before* +Three things are worth pinning. That an oversized folder is refused *before* `_openDownloadTarget`, because a save dialog for an archive that will never be -written is worse than no dialog at all. And that a folder at exactly the limit +written is worse than no dialog at all. That a folder at exactly the limit still goes through, since an off-by-one here silently costs a whole megabyte -of allowance and nobody would ever notice. +of allowance and nobody would ever notice. And that the two limits in play do +not contradict each other: ZIP_MAX_BYTES (512 MB) bounds the archive, while +MEMORY_CEILING (100 MB, test_memory_ceiling.py) 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 `confirm()` that offers the +build-in-memory path therefore only ever appears below the ceiling. """ import json @@ -30,7 +35,7 @@ pytestmark = pytest.mark.skipif( MIB = 1024 * 1024 -def _run(total_bytes, tmp_path): +def _run(total_bytes, tmp_path, picker=False): """ Call downloadDirectory over one folder holding `total_bytes`, and report what it did: the errors it set, how many times it put a question to the @@ -44,6 +49,7 @@ def _run(total_bytes, tmp_path): (tmp_path / "package.json").write_text('{"type":"module"}') script = tmp_path / "case.mjs" + picker_js = "true" if picker else "false" script.write_text(f""" const store = new Map(); globalThis.localStorage = {{ @@ -60,6 +66,17 @@ const out = {{ errors: [], started: 0, asked: 0 }}; // asks first. Answering yes is what lets the at-the-limit case get as far as // starting a transfer, and `asked` is how the refusal proves it never did. globalThis.confirm = () => {{ out.asked += 1; return true; }}; +// With `picker`, the browser can stream to a file the person chooses, which is +// the only legal route for an archive over MEMORY_CEILING. Never exercised — +// the stubbed `transfers.start` below does not run the job — it just has to be +// a target rather than null. +if ({picker_js}) {{ + window.showSaveFilePicker = async () => ({{ + name: 'album.zip', + createWritable: async () => ({{ write: async () => {{}}, close: async () => {{}}, + abort: async () => {{}} }}), + }}); +}} const M = await import('{(sandbox / "file-utils.js").as_posix()}'); @@ -101,7 +118,37 @@ def test_an_oversized_folder_is_refused_before_anything_opens(tmp_path): def test_a_folder_exactly_at_the_limit_still_downloads(tmp_path): - """The bound is inclusive: `> ZIP_MAX_BYTES`, not `>=`.""" - result = _run(512 * MIB, tmp_path) + """The bound is inclusive: `> ZIP_MAX_BYTES`, not `>=`. + + Given somewhere to stream to, because 512 MB is five times MEMORY_CEILING + and building it in the page is no longer a route this code will take. That + is what the next test is about; this one is still only about the off-by-one. + """ + result = _run(512 * MIB, tmp_path, picker=True) assert result["errors"] == [] assert result["started"] == 1 + assert result["asked"] == 0, "nothing is built in memory when it can stream" + + +def test_a_zip_over_the_memory_ceiling_is_refused_when_nothing_streams(tmp_path): + """ + Between the two limits — larger than the page may hold, smaller than the + archive limit — and no way to stream it. Before the ceiling existed this + asked "build it in memory?" and, on yes, held 400 MB in the tab. + + The refusal names the memory ceiling, not the zip limit: quoting 512 MB at + someone whose folder is under 512 MB would be a message about the wrong + rule. + """ + result = _run(400 * MIB, tmp_path) + assert result["started"] == 0 + assert result["asked"] == 0, ( + "the person must not be offered a build-in-memory path above the ceiling") + assert result["errors"] and "group.zip_too_large" not in result["errors"][0] + + +def test_a_small_folder_may_still_be_built_in_memory(tmp_path): + """The floor is intact below the ceiling — that is what it is for.""" + result = _run(4 * MIB, tmp_path) + assert result["errors"] == [] + assert result["asked"] == 1 and result["started"] == 1 |