diff options
14 files changed, 395 insertions, 16 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js index 241d761..a942fd5 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js @@ -47,15 +47,64 @@ const CHUNK_SIZE = 1024 * 1024; // that hesitates, and looks like a hang while it is quiet. const PIPELINE_WINDOW = 8; +// The most this code will ever collect in the page. +// +// Below every streaming target there is a floor: `pipelinedDownload` with no +// `writable` allocates `new Array(totalChunks)` and keeps every decrypted +// chunk, and `_saveBlob` hands the lot to the browser. That floor is fine for +// something small and is a dead tab for a film. It had **no upper bound**: the +// `!window.showSaveFilePicker` branch below returned null at any size, so on a +// browser without the File System Access API (Firefox, Safari) a 20 GB film +// went to RAM whenever the service-worker path did not answer — which happens +// for ordinary reasons (an uncontrolled page, a stream that cannot be +// transferred, the 8 s timeout). Nothing logged, nothing refused; the symptom +// was the tab dying, with no error attributable to this code. +// +// So: above this, there is no floor. A refusal naming what happened is +// recoverable and a dead tab is not. `CLAUDE.md`'s standing lesson is that a +// fallback chain reaches its floor silently — this is that floor being given a +// bottom. +const MEMORY_CEILING = 100 * 1024 * 1024; + +/** + * Thrown instead of falling through to the in-memory floor. + * + * This should now be unreachable in ordinary use: the streamed path is primed + * at application start and retried on demand, so a browser with a service + * worker has somewhere to write whatever the size. If it is ever raised, the + * reason the streamed path declined is appended — untranslated, because it is a + * diagnostic and a vague failure is what made the original bug invisible. + */ +class TooLargeForMemoryError extends Error { + constructor(filename, size) { + const why = downloads.lastStreamFailure(); + super(t('download.too_large_for_memory', { + name: filename, size: formatSize(size), limit: formatSize(MEMORY_CEILING), + }) + (why ? ` (${why})` : '')); + this.name = 'TooLargeForMemoryError'; + } +} + /** * Open somewhere to write, honouring the user's download setting. * * Returns a target ({writable, name}), null for "no stream available — collect * it and hand the browser a blob", or false for "the person dismissed the * dialog", which is not an error and must not start a transfer. + * + * **Never returns null above MEMORY_CEILING.** Every `return null` below is + * guarded by `_memoryFloor`, which throws instead. A fourth fallback added + * later must go through it too — `test_memory_ceiling.py` fails the build if a + * bare `return null` appears in this function. */ async function _openDownloadTarget(filename, size = 0, pickerOpts = {}, swSize = size) { + // "Collect it in the page", or a refusal when that would be too much. + const _memoryFloor = () => { + if (size > MEMORY_CEILING) throw new TooLargeForMemoryError(filename, size); + return null; + }; + // On a desktop build this is the whole answer, and it comes first. // // The two browser paths below are both unavailable there — `showDirectoryPicker` @@ -87,14 +136,27 @@ async function _openDownloadTarget(filename, size = 0, pickerOpts = {}, // write, which is how this works at all in Firefox: the alternative there is // to collect gigabytes in a tab. It goes to the browser's own download // folder, without a dialog, which is what "save automatically" meant. - if (downloads.getMode() === 'auto') { + // + // Tried in "ask" mode too when the file is large and this browser has no Save + // As of its own. The mode is about whether to show a dialog; it was never + // meant to decide whether a 20 GB film can be downloaded at all, and on + // Firefox and Safari — where `showSaveFilePicker` does not exist — skipping + // this block left nothing but the in-memory floor. A preference must not cost + // a capability. + const canPick = typeof window.showSaveFilePicker === 'function'; + if (downloads.getMode() === 'auto' || (size > MEMORY_CEILING && !canPick)) { const streamed = await downloads.openStreamedDownload(filename, swSize); if (streamed) return streamed; - // Nothing to stream to: small enough for memory, and no dialog. - if (size < downloads.BLOB_LIMIT) return null; + // Nothing to stream to: small enough for memory, and no dialog. The old + // comparison here was against downloads.BLOB_LIMIT (512 MB), five times + // this ceiling — and it was the *only* size test in the whole chain, with + // the branch below it unguarded. + if (size <= MEMORY_CEILING) return _memoryFloor(); } - if (!window.showSaveFilePicker) return null; + // No File System Access API — Firefox, Safari. This is the branch that used + // to return null at any size. + if (!canPick) return _memoryFloor(); try { const handle = await window.showSaveFilePicker({ suggestedName: filename, ...pickerOpts, @@ -210,7 +272,20 @@ async function pipelinedDownload(transport, gekKey, fileId, totalChunks, onChunk * download button — both just want "get this entry to disk". */ async function downloadEntry(transfers, transport, gek, entry) { - const target = await _openDownloadTarget(entry.name, entry.size); + let target; + try { + target = await _openDownloadTarget(entry.name, entry.size); + } catch (err) { + // The refusal belongs in the transfers panel, not in a console nobody + // opens: that is where someone who just clicked Download is looking, and a + // failed row naming the reason is the whole point of refusing rather than + // filling the tab. Started only to be failed, deliberately. + transfers.start({ + kind: 'download', name: entry.name, total: entry.size, transport, + run: async () => { throw err; }, + }); + return; + } if (target === false) return; // the picker was dismissed const openRef = { url: null }; @@ -292,10 +367,19 @@ async function downloadDirectory(transfers, transport, gek, entries, dir, { setE // totalBytes decides how this is delivered, but it is not the archive's // size — headers and the central directory come on top — so it is not // announced as a Content-Length that the download would then miss. - const target = await _openDownloadTarget(suggested, totalBytes, { - types: [{ description: 'ZIP archive', - accept: { 'application/zip': ['.zip'] } }], - }, 0); + let target; + try { + target = await _openDownloadTarget(suggested, totalBytes, { + types: [{ description: 'ZIP archive', + accept: { 'application/zip': ['.zip'] } }], + }, 0); + } catch (err) { + // Reported beside the folder that was clicked, like zip_too_large just + // above — this function is called in a loop over a selection, and the + // sibling folders must still download. + setError(err.message); + return; + } if (target === false) return; if (!target && !confirm(t('group.zip_no_stream', { size: formatSize(totalBytes), name: suggested, @@ -351,6 +435,7 @@ async function downloadDirectory(transfers, transport, gek, entries, dir, { setE export { FILE_ICONS, formatSize, formatDate, PREVIEWABLE_TEXT, canPreview, CHUNK_SIZE, ZIP_MAX_BYTES, + MEMORY_CEILING, TooLargeForMemoryError, _openDownloadTarget, _saveBlob, pipelinedDownload, downloadEntry, downloadDirectory, }; diff --git a/packages/meshbay-hub/src/meshbay_hub/static/files-app.js b/packages/meshbay-hub/src/meshbay_hub/static/files-app.js index 4947f8c..36c5af3 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/files-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/files-app.js @@ -6,7 +6,7 @@ import { Icon } from './icon.js'; import { entriesUnder } from './zipstream.js'; import { transfers } from './transfers.js'; import { - FILE_ICONS, formatSize, formatDate, canPreview, CHUNK_SIZE, + FILE_ICONS, formatSize, formatDate, canPreview, CHUNK_SIZE, MEMORY_CEILING, pipelinedDownload, downloadEntry, downloadDirectory as sharedDownloadDirectory, } from './file-utils.js'; @@ -598,6 +598,24 @@ function FilePreview({ entry, transportRef, gekRef, onClose, onDownload }) { useEffect(() => { let cancelled = false; const load = async () => { + // Nothing here streams: a preview is decrypted whole, held as an array of + // chunks, and turned into a blob. That is right for a page of text and a + // photograph, and it is a dead tab for the things that also reach here — + // a scanned PDF, a multi-gigabyte .csv or .log. There was no size test at + // all, and the text branch is the sharpest illustration: it decoded the + // entire file and then kept 500 000 characters of it. + // + // Films and music never arrive (group-page.js's onPreview routes video to + // the MSE player and audio to the music queue), so this guard is only ever + // met by a document somebody clicked without knowing how big it was. It + // offers the download instead, which does stream. + if (entry.size > MEMORY_CEILING) { + setError(t('preview.too_large', { + size: formatSize(entry.size), limit: formatSize(MEMORY_CEILING), + })); + setPhase('error'); + return; + } const transport = transportRef.current; if (!transport || !transport.connected) { setError(t('video.err_transport')); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js index 97a835d..1ebab8b 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js @@ -216,6 +216,8 @@ export default { 'video.close': 'Schließen (Esc)', 'preview.pdf_fallback': 'Dieser Browser zeigt das PDF nicht direkt an. Laden Sie es ' + 'stattdessen herunter — entschlüsselt wurde es ohnehin hier.', + 'preview.too_large': 'Diese Datei ist {size} groß, mehr als diese Seite im Arbeitsspeicher halten kann ({limit}). Laden Sie sie stattdessen herunter — ein Download wird direkt auf die Festplatte geschrieben.', + 'download.too_large_for_memory': '„{name}“ ist {size} groß. Dieser Browser kann eine Datei dieser Größe nur speichern, indem er sie direkt auf die Festplatte schreibt, und das ist hier nicht möglich — er müsste die ganze Datei im Arbeitsspeicher halten. Verwenden Sie die Desktop-App oder Chrome bzw. Edge.', 'group.upload_indexing': 'wird indiziert …', 'video.err_transport': 'Transport nicht verbunden', 'video.err_mse': 'Codec wird für das Streaming nicht unterstützt: {codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js index c55a702..d85f51b 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js @@ -214,6 +214,8 @@ export default { 'video.from_start': "Start from the beginning", 'video.close': 'Close (Esc)', 'preview.pdf_fallback': 'This browser will not display the PDF inline. Download it instead — it was decrypted here either way.', + 'preview.too_large': 'This file is {size}, more than this page can hold in memory ({limit}). Download it instead — a download is written straight to disk.', + 'download.too_large_for_memory': '"{name}" is {size}. This browser can only save a file that large by streaming it to disk, and it has no way to do that here — it would have to hold the whole file in memory. Use the desktop app, or Chrome or Edge.', 'group.upload_indexing': 'indexing…', 'video.err_transport': 'Transport not connected', 'video.err_mse': 'Codec not supported for streaming: {codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js index 9ce3900..bfe4112 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js @@ -214,6 +214,8 @@ export default { 'video.close': 'Cerrar (Esc)', 'preview.pdf_fallback': 'Este navegador no mostrará el PDF integrado. Descárguelo ' + 'en su lugar — en cualquier caso se descifró aquí.', + 'preview.too_large': 'Este archivo ocupa {size}, más de lo que esta página puede mantener en memoria ({limit}). Descárguelo en su lugar — una descarga se escribe directamente en disco.', + 'download.too_large_for_memory': '«{name}» ocupa {size}. Este navegador solo puede guardar un archivo así transmitiéndolo al disco, y aquí no puede hacerlo — tendría que mantener el archivo entero en memoria. Use la aplicación de escritorio, o Chrome o Edge.', 'group.upload_indexing': 'indexando…', 'video.err_transport': 'Transporte no conectado', 'video.err_mse': 'Códec no compatible con la reproducción en continuo: {codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js index d612f71..9addec5 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js @@ -215,6 +215,8 @@ export default { 'video.close': 'Fermer (Échap)', 'preview.pdf_fallback': 'Ce navigateur n’affichera pas le PDF directement. ' + 'Téléchargez-le plutôt — il a été déchiffré ici dans les deux cas.', + 'preview.too_large': 'Ce fichier fait {size}, plus que cette page ne peut garder en mémoire ({limit}). Téléchargez-le plutôt — un téléchargement est écrit directement sur le disque.', + 'download.too_large_for_memory': '« {name} » fait {size}. Ce navigateur ne peut enregistrer un fichier de cette taille qu\'en l\'écrivant au fil de l\'eau sur le disque, ce qu\'il ne peut pas faire ici — il devrait garder le fichier entier en mémoire. Utilisez l\'application de bureau, ou Chrome ou Edge.', 'group.upload_indexing': 'indexation…', 'video.err_transport': 'Transport non connecté', 'video.err_mse': 'Codec non pris en charge pour la diffusion : {codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js index c61b8e5..fe76b9e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js @@ -215,6 +215,8 @@ export default { 'video.close': 'Chiudi (Esc)', 'preview.pdf_fallback': 'Questo browser non mostrerà il PDF nella pagina. Lo scarichi ' + 'invece — in ogni caso è stato decifrato qui.', + 'preview.too_large': 'Questo file è di {size}, più di quanto questa pagina possa tenere in memoria ({limit}). Lo scarichi invece — un download viene scritto direttamente su disco.', + 'download.too_large_for_memory': '«{name}» è di {size}. Questo browser può salvare un file di queste dimensioni solo scrivendolo su disco man mano, e qui non può farlo — dovrebbe tenere l’intero file in memoria. Usi l’applicazione desktop, oppure Chrome o Edge.', 'group.upload_indexing': 'indicizzazione…', 'video.err_transport': 'Trasporto non connesso', 'video.err_mse': 'Codec non supportato per lo streaming: {codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js index 20c5fd1..272be73 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js @@ -212,6 +212,8 @@ export default { 'video.close': '閉じる（Esc）', 'preview.pdf_fallback': 'このブラウザーはページ内に PDF を表示しません。' + 'ダウンロードしてご覧ください。いずれにせよ復号はここで行われています。', + 'preview.too_large': 'このファイルは {size} で、このページがメモリに保持できる上限（{limit}）を超えています。代わりにダウンロードしてください。ダウンロードはディスクに直接書き込まれます。', + 'download.too_large_for_memory': '「{name}」は {size} です。このブラウザーでこの大きさのファイルを保存するにはディスクへ逐次書き出すしかありませんが、ここではそれができません — ファイル全体をメモリに保持することになります。デスクトップアプリ、または Chrome か Edge をお使いください。', 'group.upload_indexing': 'インデックスを作成中…', 'video.err_transport': 'トランスポートが接続されていません', 'video.err_mse': 'ストリーミング再生に対応していないコーデックです：{codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js index 4b992ae..76f3586 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js @@ -216,6 +216,8 @@ export default { 'video.close': 'Sluiten (Esc)', 'preview.pdf_fallback': 'Deze browser toont de PDF niet in de pagina zelf. Download ' + 'hem in plaats daarvan — ontsleuteld werd hij hoe dan ook hier.', + 'preview.too_large': 'Dit bestand is {size}, meer dan deze pagina in het geheugen kan houden ({limit}). Download het in plaats daarvan — een download wordt rechtstreeks naar schijf geschreven.', + 'download.too_large_for_memory': '“{name}” is {size}. Deze browser kan een bestand van die omvang alleen opslaan door het meteen naar schijf te schrijven, en dat kan hier niet — het hele bestand zou in het geheugen moeten. Gebruik de desktop-app, of Chrome of Edge.', 'group.upload_indexing': 'indexeren…', 'video.err_transport': 'Transport niet verbonden', 'video.err_mse': 'Codec wordt niet ondersteund voor streamen: {codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js index 5389220..dd05487 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js @@ -221,6 +221,8 @@ export default { 'video.close': 'Zamknij (Esc)', 'preview.pdf_fallback': 'Ta przeglądarka nie wyświetli pliku PDF na stronie. Proszę ' + 'go pobrać — i tak został odszyfrowany tutaj.', + 'preview.too_large': 'Ten plik ma {size}, więcej niż ta strona może utrzymać w pamięci ({limit}). Proszę go zamiast tego pobrać — pobieranie jest zapisywane wprost na dysk.', + 'download.too_large_for_memory': '„{name}” ma {size}. Ta przeglądarka może zapisać plik tej wielkości tylko strumieniowo na dysk, a tutaj nie ma takiej możliwości — musiałaby utrzymać cały plik w pamięci. Proszę użyć aplikacji desktopowej albo przeglądarki Chrome lub Edge.', 'group.upload_indexing': 'indeksowanie…', 'video.err_transport': 'Transport nie jest połączony', 'video.err_mse': 'Kodek nieobsługiwany przy odtwarzaniu strumieniowym: {codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js index d72a6e7..4632d36 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js @@ -216,6 +216,8 @@ export default { 'video.close': 'Fechar (Esc)', 'preview.pdf_fallback': 'Este navegador não exibirá o PDF na própria página. Baixe ' + 'o arquivo — de todo modo ele foi descriptografado aqui.', + 'preview.too_large': 'Este arquivo tem {size}, mais do que esta página consegue manter na memória ({limit}). Baixe-o em vez disso — um download é gravado direto no disco.', + 'download.too_large_for_memory': '"{name}" tem {size}. Este navegador só consegue salvar um arquivo desse tamanho gravando-o direto no disco, e aqui ele não tem como — precisaria manter o arquivo inteiro na memória. Use o aplicativo para computador, ou Chrome ou Edge.', 'group.upload_indexing': 'indexando…', 'video.err_transport': 'Transporte não conectado', 'video.err_mse': 'Codec sem suporte para transmissão: {codec}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js index c672a13..b6ff3c9 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js @@ -209,6 +209,8 @@ export default { 'video.from_start': "从头开始播放", 'video.close': '关闭（Esc）', 'preview.pdf_fallback': '此浏览器不会在页面内显示该 PDF。请改为下载——无论如何它都已在本地解密。', + 'preview.too_large': '该文件为 {size}，超出本页面可在内存中保存的上限（{limit}）。请改为下载——下载会直接写入磁盘。', + 'download.too_large_for_memory': '“{name}”为 {size}。此浏览器只能通过边下边写入磁盘来保存这么大的文件，而这里无法做到——它将不得不把整个文件放在内存中。请使用桌面应用，或 Chrome、Edge。', 'group.upload_indexing': '建立索引中…', 'video.err_transport': '传输未连接', 'video.err_mse': '该编解码器不支持流式播放：{codec}', 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") 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 |