openvibers/openvibe.host Commits
Decompress uploads on the threadpool, not the event loop, with the same cap and errors (#22)
Changes
@@@ -286,5 +286,5 @@ the history of any site.| Cookie bomb across tenants and the dashboard (no PSL) | low (per-visitor nuisance; takedown exists) | owner decision (§5) || No abuse mailbox/owner | medium (process, not code) | owner || Objects not backed up | medium for tenants (alpha, documented) | follow-up: add the object store to the backup run |--| `gunzipSync` blocks the event loop per upload | low (bounded) | follow-up if uploads grow |++| Decompressing an upload blocked the event loop (`gunzipSync`) | done: async `zlib.gunzip` on the libuv threadpool (`server/artifacts/archive.js`), same `maxOutputLength` cap | done || Takedown routes not in released `openvibe-contracts` `host.site.manage.implementedBy` | none at runtime (the contract check passes) | Contracts: next release (docs/launch.md) |@@@ -6,9 +6,13 @@* The reader never touches the filesystem: entries are parsed from a Buffer and nothing is* extracted. Symbolic links, hard links, devices, FIFOs, sparse and other special entries are* refused (the whole deploy fails), never skipped silently. Decompression is capped-- * (maxOutputLength), so a gzip bomb stops at the limit instead of filling memory.++ * (maxOutputLength), so a gzip bomb stops at the limit instead of filling memory; it runs on the++ * libuv threadpool (async zlib), so a large upload does not block the event loop.*/const zlib = require('zlib');++const { promisify } = require('util');++++const gunzip = promisify(zlib.gunzip);class ArchiveError extends Error {constructor(code, message, status = 422) { super(message); this.code = code; this.status = status; }@@@ -60,13 +64,13 @@ const TYPE_NAMES = { 1: 'a hard link', 2: 'a symbolic link', 3: 'a character dev* buf: the uploaded bytes (gzip or plain tar). limits: { maxTotalBytes, maxFiles, maxFileBytes }.* -> { entries: [{ path, data }], directories: n, compressed: bool }*/--function readArchive(buf, limits) {++async function readArchive(buf, limits) {let tar = buf;const compressed = buf.length >= 2 && buf[0] === 0x1f && buf[1] === 0x8b;if (compressed) {const cap = limits.maxTotalBytes + (limits.maxFiles + 16) * 3 * BLOCK + 1024 * 1024;try {-- tar = zlib.gunzipSync(buf, { maxOutputLength: cap });++ tar = await gunzip(buf, { maxOutputLength: cap });} catch (err) {if (err && (err.code === 'ERR_BUFFER_TOO_LARGE' || err instanceof RangeError)) {throw new ArchiveError('deploy.too_large', 'the archive expands beyond the size this project may deploy', 413);@@@ -151,7 +151,7 @@ async function readUpload(req, { maxUploadBytes, maxUnpackedBytes = Infinity, liif (rawArchive) {source = 'archive';let r;-- try { r = readArchive(rawArchive, { ...limits, maxTotalBytes: Math.min(limits.maxTotalBytes, maxUnpackedBytes) }); } catch (err) {++ try { r = await readArchive(rawArchive, { ...limits, maxTotalBytes: Math.min(limits.maxTotalBytes, maxUnpackedBytes) }); } catch (err) {if (err instanceof ArchiveError) { const e = new UploadError(err.status, err.code, err.message); e.fields = fields; e.source = 'archive'; throw e; }throw err;}@@@ -10,6 +10,7 @@ const path = require('path');const zlib = require('zlib');const { boot, check, done } = require('./stageb/boot');const { tarball } = require('./stageb/tar');++const { readArchive } = require('../server/artifacts/archive');(async () => {const t = await boot();@@@ -101,6 +102,22 @@ const { tarball } = require('./stageb/tar');await t.api('PUT', `/api/v1/projects/${project.id}/quota`, { as: t.user('staff2', { role: 'admin' }), json: { storage_bytes: null } });});++ await check('a large decompression leaves the event loop responsive', async () => {++ const size = 32 * 1024 * 1024;++ const raw = tarball([{ name: 'big.txt', content: Buffer.alloc(size, 97) }]);++ assert.ok(raw.length < 1024 * 1024, 'the archive is small on the wire');++ let resolved = false;++ const pending = readArchive(raw, { maxTotalBytes: 64 * 1024 * 1024, maxFiles: 10, maxFileBytes: 64 * 1024 * 1024 })++ .then((r) => { resolved = true; return r; });++ assert.strictEqual(typeof pending.then, 'function', 'readArchive is asynchronous');++ await new Promise((r) => setTimeout(r, 0));++ assert.strictEqual(resolved, false, 'a timer fired while the archive was still decompressing');++ const parsed = await pending;++ assert.strictEqual(parsed.compressed, true);++ assert.strictEqual(parsed.entries.length, 1);++ assert.strictEqual(parsed.entries[0].data.length, size);++ });++await check('multipart filenames go through the same rules', async () => {const boundary = 'xxBOUNDARYxx';const part = (filename, content) => `--${boundary}\r\nContent-Disposition: form-data; name="files"; filename="${filename}"\r\nContent-Type: text/html\r\n\r\n${content}\r\n`;
@@ -286,5 +286,5 @@ the history of any site.
@@ -286,5 +286,5 @@ the history of any site.
| Cookie bomb across tenants and the dashboard (no PSL) | low (per-visitor nuisance; takedown exists) | owner decision (§5) |
| Cookie bomb across tenants and the dashboard (no PSL) | low (per-visitor nuisance; takedown exists) | owner decision (§5) |
| No abuse mailbox/owner | medium (process, not code) | owner |
| No abuse mailbox/owner | medium (process, not code) | owner |
| Objects not backed up | medium for tenants (alpha, documented) | follow-up: add the object store to the backup run |
| Objects not backed up | medium for tenants (alpha, documented) | follow-up: add the object store to the backup run |
-| `gunzipSync` blocks the event loop per upload | low (bounded) | follow-up if uploads grow |
+| Decompressing an upload blocked the event loop (`gunzipSync`) | done: async `zlib.gunzip` on the libuv threadpool (`server/artifacts/archive.js`), same `maxOutputLength` cap | done |
| Takedown routes not in released `openvibe-contracts` `host.site.manage.implementedBy` | none at runtime (the contract check passes) | Contracts: next release (docs/launch.md) |
| Takedown routes not in released `openvibe-contracts` `host.site.manage.implementedBy` | none at runtime (the contract check passes) | Contracts: next release (docs/launch.md) |
@@ -6,9 +6,13 @@
@@ -6,9 +6,13 @@
* The reader never touches the filesystem: entries are parsed from a Buffer and nothing is
* The reader never touches the filesystem: entries are parsed from a Buffer and nothing is
* extracted. Symbolic links, hard links, devices, FIFOs, sparse and other special entries are
* extracted. Symbolic links, hard links, devices, FIFOs, sparse and other special entries are
* refused (the whole deploy fails), never skipped silently. Decompression is capped
* refused (the whole deploy fails), never skipped silently. Decompression is capped
- * (maxOutputLength), so a gzip bomb stops at the limit instead of filling memory.
+ * (maxOutputLength), so a gzip bomb stops at the limit instead of filling memory; it runs on the
+ * libuv threadpool (async zlib), so a large upload does not block the event loop.
*/
*/
const zlib = require('zlib');
const zlib = require('zlib');
+const { promisify } = require('util');
+
+const gunzip = promisify(zlib.gunzip);
class ArchiveError extends Error {
class ArchiveError extends Error {
constructor(code, message, status = 422) { super(message); this.code = code; this.status = status; }
constructor(code, message, status = 422) { super(message); this.code = code; this.status = status; }
@@ -60,13 +64,13 @@ const TYPE_NAMES = { 1: 'a hard link', 2: 'a symbolic link', 3: 'a character dev
@@ -60,13 +64,13 @@ const TYPE_NAMES = { 1: 'a hard link', 2: 'a symbolic link', 3: 'a character dev
* buf: the uploaded bytes (gzip or plain tar). limits: { maxTotalBytes, maxFiles, maxFileBytes }.
* buf: the uploaded bytes (gzip or plain tar). limits: { maxTotalBytes, maxFiles, maxFileBytes }.
* -> { entries: [{ path, data }], directories: n, compressed: bool }
* -> { entries: [{ path, data }], directories: n, compressed: bool }
*/
*/
-function readArchive(buf, limits) {
+async function readArchive(buf, limits) {
let tar = buf;
let tar = buf;
const compressed = buf.length >= 2 && buf[0] === 0x1f && buf[1] === 0x8b;
const compressed = buf.length >= 2 && buf[0] === 0x1f && buf[1] === 0x8b;
if (compressed) {
if (compressed) {
const cap = limits.maxTotalBytes + (limits.maxFiles + 16) * 3 * BLOCK + 1024 * 1024;
const cap = limits.maxTotalBytes + (limits.maxFiles + 16) * 3 * BLOCK + 1024 * 1024;
try {
try {
- tar = zlib.gunzipSync(buf, { maxOutputLength: cap });
+ tar = await gunzip(buf, { maxOutputLength: cap });
} catch (err) {
} catch (err) {
if (err && (err.code === 'ERR_BUFFER_TOO_LARGE' || err instanceof RangeError)) {
if (err && (err.code === 'ERR_BUFFER_TOO_LARGE' || err instanceof RangeError)) {
throw new ArchiveError('deploy.too_large', 'the archive expands beyond the size this project may deploy', 413);
throw new ArchiveError('deploy.too_large', 'the archive expands beyond the size this project may deploy', 413);
@@ -151,7 +151,7 @@ async function readUpload(req, { maxUploadBytes, maxUnpackedBytes = Infinity, li
@@ -151,7 +151,7 @@ async function readUpload(req, { maxUploadBytes, maxUnpackedBytes = Infinity, li
if (rawArchive) {
if (rawArchive) {
source = 'archive';
source = 'archive';
let r;
let r;
- try { r = readArchive(rawArchive, { ...limits, maxTotalBytes: Math.min(limits.maxTotalBytes, maxUnpackedBytes) }); } catch (err) {
+ try { r = await readArchive(rawArchive, { ...limits, maxTotalBytes: Math.min(limits.maxTotalBytes, maxUnpackedBytes) }); } catch (err) {
if (err instanceof ArchiveError) { const e = new UploadError(err.status, err.code, err.message); e.fields = fields; e.source = 'archive'; throw e; }
if (err instanceof ArchiveError) { const e = new UploadError(err.status, err.code, err.message); e.fields = fields; e.source = 'archive'; throw e; }
throw err;
throw err;
}
}
@@ -10,6 +10,7 @@ const path = require('path');
@@ -10,6 +10,7 @@ const path = require('path');
const zlib = require('zlib');
const zlib = require('zlib');
const { boot, check, done } = require('./stageb/boot');
const { boot, check, done } = require('./stageb/boot');
const { tarball } = require('./stageb/tar');
const { tarball } = require('./stageb/tar');
+const { readArchive } = require('../server/artifacts/archive');
(async () => {
(async () => {
const t = await boot();
const t = await boot();
@@ -101,6 +102,22 @@ const { tarball } = require('./stageb/tar');
@@ -101,6 +102,22 @@ const { tarball } = require('./stageb/tar');
await t.api('PUT', `/api/v1/projects/${project.id}/quota`, { as: t.user('staff2', { role: 'admin' }), json: { storage_bytes: null } });
await t.api('PUT', `/api/v1/projects/${project.id}/quota`, { as: t.user('staff2', { role: 'admin' }), json: { storage_bytes: null } });
});
});
+ await check('a large decompression leaves the event loop responsive', async () => {
+ const size = 32 * 1024 * 1024;
+ const raw = tarball([{ name: 'big.txt', content: Buffer.alloc(size, 97) }]);
+ assert.ok(raw.length < 1024 * 1024, 'the archive is small on the wire');
+ let resolved = false;
+ const pending = readArchive(raw, { maxTotalBytes: 64 * 1024 * 1024, maxFiles: 10, maxFileBytes: 64 * 1024 * 1024 })
+ .then((r) => { resolved = true; return r; });
+ assert.strictEqual(typeof pending.then, 'function', 'readArchive is asynchronous');
+ await new Promise((r) => setTimeout(r, 0));
+ assert.strictEqual(resolved, false, 'a timer fired while the archive was still decompressing');
+ const parsed = await pending;
+ assert.strictEqual(parsed.compressed, true);
+ assert.strictEqual(parsed.entries.length, 1);
+ assert.strictEqual(parsed.entries[0].data.length, size);
+ });
+
await check('multipart filenames go through the same rules', async () => {
await check('multipart filenames go through the same rules', async () => {
const boundary = 'xxBOUNDARYxx';
const boundary = 'xxBOUNDARYxx';
const part = (filename, content) => `--${boundary}\r\nContent-Disposition: form-data; name="files"; filename="${filename}"\r\nContent-Type: text/html\r\n\r\n${content}\r\n`;
const part = (filename, content) => `--${boundary}\r\nContent-Disposition: form-data; name="files"; filename="${filename}"\r\nContent-Type: text/html\r\n\r\n${content}\r\n`;