Wire --body-limit into createServer; real CONTENT_LENGTH for chunked git pushes (JavaScriptSolidServer#562)

melvincarvalho · web-flow · commit e94a18eb60bf · 2026-06-12T05:31:12.000+02:00
* fix: wire --body-limit into createServer; real CONTENT_LENGTH for chunked git pushes Two halves of the same user-facing bug (JavaScriptSolidServer#561) — 'git push of a pack over 10MB cannot succeed no matter what the operator configures': 1. bin/jss.js start parsed --body-limit / JSS_BODY_LIMIT into config.bodyLimit and then dropped it — the hand-written options object passed to createServer() omitted bodyLimit, so every CLI-started server enforced the 10MiB default. The JavaScriptSolidServer#474 knob was dead wiring; only programmatic createServer({ bodyLimit }) worked. 2. With the limit actually raised, pushes > 1MiB still died: git sends packs over http.postBuffer (1MiB) with Transfer-Encoding: chunked and no Content-Length, and the git handler set the CGI's CONTENT_LENGTH from the absent header ('0'), so http-backend read zero bytes and receive-pack failed with 'the remote end hung up unexpectedly'. Use the buffered body's real length instead. Regression tests (test/body-limit-cli.test.js) spawn the real CLI and assert the limit reaches Fastify via flag and env — 2 of 3 fail without fix 1. Verified end-to-end: 'jss install solid-chat/app=chat JavaScriptSolidServer/git' (13.6MB / >1MiB packs, previously impossible) now completes 2/2 with extracted working trees serving 200. Full suite: 961 pass, 0 fail. Fixes JavaScriptSolidServer#561 * review fix (JavaScriptSolidServer#562): bound stopCli's final await; document why the attach can't race Copilot flagged a race between the exitCode check and attaching the 'exit' listener. That specific interleaving can't occur — both are in the same synchronous block, and Node can't emit 'exit' between two synchronous statements (if the event already fired, exitCode is non-null and we return before attaching). Documented in a comment. The comment's spirit stands though: the final `await gone` was unbounded, so a pathological unkillable child (stuck in uninterruptible I/O) would hang the suite forever. Capped with a 2s race — SIGKILL can't be caught, so 'exit' follows promptly in every realistic case; the bound is belt-and-braces. 3/3 passing.
diff --git a/bin/jss.js b/bin/jss.js
@@ -190,6 +190,10 @@ program
       const server = createServer({
         port: config.port,
         host: config.host,
+        // Wire the parsed --body-limit / JSS_BODY_LIMIT value through —
+        // omitting it here silently pinned every CLI-started server to
+        // the 10MB default and made the #474 knob dead wiring (#561).
+        bodyLimit: config.bodyLimit,
         logger: config.logger,
         conneg: config.conneg,
         notifications: config.notifications,
diff --git a/src/handlers/git.js b/src/handlers/git.js
@@ -279,7 +279,15 @@ export async function handleGit(request, reply) {
     CONTENT_TYPE: request.headers['content-type'] || '',
     QUERY_STRING: queryString,
     REMOTE_USER: request.webId || '',           // Pass authenticated user
-    CONTENT_LENGTH: request.headers['content-length'] || '0',
+    // Use the buffered body's real length, not the Content-Length header:
+    // git sends packs larger than http.postBuffer (1 MiB default) with
+    // Transfer-Encoding: chunked and NO Content-Length, so the header
+    // fallback told http-backend "0 bytes" and receive-pack died with
+    // "the remote end hung up unexpectedly" on any push > 1 MiB (#561).
+    // Fastify has already buffered the full body either way.
+    CONTENT_LENGTH: request.body?.length
+      ? String(request.body.length)
+      : (request.headers['content-length'] || '0'),
   };
 
   // For regular repositories, set GIT_DIR
diff --git a/test/body-limit-cli.test.js b/test/body-limit-cli.test.js
@@ -0,0 +1,130 @@
+/**
+ * CLI wiring for --body-limit / JSS_BODY_LIMIT (#561).
+ *
+ * test/body-limit.test.js covers createServer({ bodyLimit }) — the
+ * programmatic path — but the CLI start command builds its createServer
+ * options object by hand, and originally omitted `bodyLimit`. Result:
+ * the #474 knob parsed fine and was then silently dropped, pinning every
+ * CLI-started server to the 10 MiB default. git pushes of packs over the
+ * default failed with 413 no matter what the operator passed (#561).
+ *
+ * These tests spawn `bin/jss.js start` as a real subprocess (same
+ * pattern as cli-flag-like-values.test.js) and assert the limit
+ * actually reaches Fastify:
+ *
+ *   - `--body-limit 1KB`  → a 2 KiB PUT is rejected with 413
+ *   - `--body-limit 5MB`  → the same PUT is NOT rejected with 413
+ *   - `JSS_BODY_LIMIT=1KB` (env, no flag) → 413, same as the flag
+ *
+ * The over-limit assertions are deliberately status-exact (413) while
+ * the under-limit assertion is only "not 413": body parsing runs before
+ * auth/WAC, so an under-limit request may still be refused later in the
+ * pipeline — that's fine, the wiring under test is the parser cap.
+ */
+
+import { describe, it, afterEach } from 'node:test';
+import assert from 'node:assert';
+import { spawn } from 'node:child_process';
+import { createServer as createNetServer } from 'node:net';
+import { fileURLToPath } from 'node:url';
+import path from 'node:path';
+import fs from 'fs-extra';
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+const BIN = path.join(__dirname, '..', 'bin', 'jss.js');
+const TEST_DATA_DIR = './test-data-body-limit-cli';
+
+let child;
+
+function freePort() {
+  return new Promise((resolve, reject) => {
+    const srv = createNetServer();
+    srv.once('error', reject);
+    srv.listen(0, '127.0.0.1', () => {
+      const { port } = srv.address();
+      srv.close(() => resolve(port));
+    });
+  });
+}
+
+async function startCli({ args = [], env = {} }) {
+  await fs.emptyDir(TEST_DATA_DIR);
+  const port = await freePort();
+  child = spawn(process.execPath, [
+    BIN, 'start',
+    '--port', String(port),
+    '--host', '127.0.0.1',
+    '--root', TEST_DATA_DIR,
+    ...args,
+  ], {
+    env: { ...process.env, ...env },
+    stdio: ['ignore', 'pipe', 'pipe'],
+  });
+  const baseUrl = `http://127.0.0.1:${port}`;
+
+  // Poll until the server answers (or the child dies / 15 s passes).
+  const deadline = Date.now() + 15_000;
+  let exited = false;
+  child.once('exit', () => { exited = true; });
+  while (Date.now() < deadline) {
+    if (exited) throw new Error('jss CLI exited before becoming ready');
+    try {
+      await fetch(baseUrl, { signal: AbortSignal.timeout(1000) });
+      return baseUrl;
+    } catch {
+      await new Promise(r => setTimeout(r, 250));
+    }
+  }
+  throw new Error('jss CLI did not become ready within 15s');
+}
+
+async function stopCli() {
+  if (!child) return;
+  const c = child;
+  child = null;
+  // The exitCode check and the once('exit') attach below are in the
+  // same synchronous block — Node can't emit 'exit' between two
+  // synchronous statements, so the listener can't miss the event (if
+  // it already fired, exitCode is non-null and we return here).
+  if (c.exitCode !== null || c.signalCode !== null) return;
+  const gone = new Promise(r => c.once('exit', r));
+  c.kill('SIGTERM');
+  await Promise.race([gone, new Promise(r => setTimeout(r, 3000))]);
+  if (c.exitCode === null && c.signalCode === null) c.kill('SIGKILL');
+  // SIGKILL can't be caught, so 'exit' follows promptly. The cap is a
+  // belt-and-braces bound so the suite can never hang on a pathological
+  // unkillable child (e.g. stuck in uninterruptible I/O).
+  await Promise.race([gone, new Promise(r => setTimeout(r, 2000))]);
+}
+
+async function putTwoKiB(baseUrl) {
+  const res = await fetch(`${baseUrl}/body-limit-probe.bin`, {
+    method: 'PUT',
+    headers: { 'Content-Type': 'application/octet-stream' },
+    body: Buffer.alloc(2048, 0x61),
+    signal: AbortSignal.timeout(5000),
+  });
+  return res.status;
+}
+
+describe('bin/jss.js — --body-limit reaches Fastify (#561)', () => {
+  afterEach(async () => {
+    await stopCli();
+    await fs.remove(TEST_DATA_DIR);
+  });
+
+  it('--body-limit 1KB rejects a 2KiB PUT with 413', async () => {
+    const baseUrl = await startCli({ args: ['--body-limit', '1KB'] });
+    assert.strictEqual(await putTwoKiB(baseUrl), 413);
+  });
+
+  it('--body-limit 5MB does not 413 a 2KiB PUT', async () => {
+    const baseUrl = await startCli({ args: ['--body-limit', '5MB'] });
+    assert.notStrictEqual(await putTwoKiB(baseUrl), 413);
+  });
+
+  it('JSS_BODY_LIMIT=1KB (env, no flag) rejects a 2KiB PUT with 413', async () => {
+    const baseUrl = await startCli({ env: { JSS_BODY_LIMIT: '1KB' } });
+    assert.strictEqual(await putTwoKiB(baseUrl), 413);
+  });
+});
