| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
…DY_LIMIT (#474) The hard-coded 10 MiB cap at src/server.js:193 blocked `git push` of established app repos (reporter hit 413 trying to install solid-chat/app, a ~30 MiB pack from 3 years of history). Operators had no way to raise it. Surface added: - `createServer({ bodyLimit })` — accepts a number (bytes) or a size string ("100MB", "1GB", …). String parsing uses the existing config.parseSize helper, so the format matches the `defaultQuota` flag already in the codebase. - CLI: `--body-limit <size>` in bin/jss.js, alongside the existing --cors-proxy-max-* family. - Env: `JSS_BODY_LIMIT` in config.envMap; routed through parseEnvValue's size-coercion branch so JSS_BODY_LIMIT=100MB becomes 104857600. - Config: `bodyLimit` field in config.defaults (default 10 MiB = previous behaviour). createServer normalises with a small ternary so any caller — number or string — gets the same bytes-going-in. Default unchanged so every existing user keeps the same behaviour. Not in this PR: a per-content-type override that lets git pushes exceed the global cap (the issue's 'optional follow-up'). Separate enhancement if anyone wants it; the global option is enough for operators of personal pods to raise their own cap. Test coverage in test/body-limit.test.js (3 cases): - numeric bodyLimit → over-size request gets 413 - string bodyLimit ("100B") → same behaviour, proves parseSize path - in-bounds request → not 413 Full suite 920/920 passing (917 + 3 new). Closes #474.
There was a problem hiding this comment.
Adds a configurable Fastify bodyLimit so operators can raise the request-body cap (notably for larger git push payloads) while keeping the default behavior unchanged (10 MiB).
Changes:
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| test/body-limit.test.js | New tests asserting Fastify enforces configured bodyLimit for numeric and size-string inputs. |
| src/server.js | Resolve bodyLimit from options/defaults and pass through to Fastify options. |
| src/config.js | Add bodyLimit default and allow JSS_BODY_LIMIT env var (size parsing). |
| bin/jss.js | Add --body-limit CLI option help/flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| * implicitly by every other test in the suite continuing to pass. | ||
| */ | ||
|
|
||
| import { describe, it, before, after, beforeEach, afterEach } from 'node:test'; |
There was a problem hiding this comment.
Done in 14f0862. Dropped beforeEach from the import list (truly unused); kept before/after since the DATA_ROOT snapshot/restore added in the paired threads now needs them.
Sorry, something went wrong.
| let server; | ||
| let baseUrl; | ||
|
|
Sorry, something went wrong.
| afterEach(async () => { | ||
| if (server) { | ||
| await server.close(); | ||
| server = null; | ||
| } | ||
| await fs.remove(TEST_DATA_DIR); | ||
| }); |
There was a problem hiding this comment.
Done in 14f0862 via the same snapshot/restore added in the paired thread. after hook restores DATA_ROOT (or deletes it if it was unset before the suite ran).
Sorry, something went wrong.
| // Resolve bodyLimit from options. Accept either a number (programmatic | ||
| // / env-parsed) or a size string ("100MB" from CLI / config files); | ||
| // parseSize handles both forms uniformly. Default matches the previous | ||
| // hard-coded 10 MiB cap. See #474. |
There was a problem hiding this comment.
Done in 14f0862. Rewrote the comment to spell out the typeof bypass and the reason — parseSize calls .match so a raw number would throw; numbers pass straight through. Wording now matches what the code actually does.
Sorry, something went wrong.
| async function startWith(bodyLimit) { | ||
| await fs.emptyDir(TEST_DATA_DIR); |
There was a problem hiding this comment.
Done in 14f0862 via the snapshot/restore added in the paired threads. before captures the pre-suite value, after restores it.
Sorry, something went wrong.
…omment
Three Copilot findings (5 inline comments — the DATA_ROOT pollution
was flagged 3 times pointing at the same fix):
1. **Unused test imports.** Dropped `before`, `after`, `beforeEach`
from the speculative import list — only `describe`, `it`,
`afterEach` are used (plus `before`, `after` are now genuinely
needed by the DATA_ROOT snapshot/restore added below, so they
stay).
2. **createServer({ root }) mutates process.env.DATA_ROOT.** This is
confirmed at src/server.js:180. Without restoration, the test
directory leaks into any subsequent test that reads DATA_ROOT.
Added a snapshot/restore pattern via before/after hooks, mirroring
what test/schnorr-complete-defensive.test.js (#539) already does.
3. **Misleading comment in server.js.** The previous wording said
parseSize 'handles both forms uniformly' — but the code actually
typeof-bypasses for numbers (parseSize would throw on a number
because it calls .match). Rewrote the comment to spell out the
bypass and the reason (parseSize takes strings only).
Full suite 920/920 still passing.
Bundles everything merged since 0.0.205 was published to npm (2026-06-07): - #539 fix(idp): handleSchnorrComplete recovers from interactionFinished throw — missing _interaction cookie now gets a clean 400 instead of a hung socket / gateway 504 (closes #412) - #540 refactor(idp): sweep the hijack-then-throw defensive pattern across all 5 interaction handlers via a shared finishInteractionDefensively helper (foundation for #526) - #542 fix: align runtime port fallbacks with the canonical 4443 default — one source of truth in config.defaults (closes #477) - #543 feat: configurable bodyLimit via createServer({ bodyLimit }), --body-limit, and JSS_BODY_LIMIT — large git pushes no longer 413 at the hard-coded 10 MiB cap (closes #474) - #546 fix(well-known-did-nostr): probe profile/card.jsonld for root-path WebIDs like https://melvin.solid.social/#me, with a cross-account guard on the subdomain fallback (closes #451)
… auto-init suite (#375) src/handlers/git.js had zero automated coverage — #371 (CORS) and #373 (multi-slash) were verified by hand. New test/git-handler.test.js pins the HTTP contract with 8 cases: - OPTIONS preflight → 200 + canonical git CORS headers (#371) - single/double/triple-slash info/refs advertise → 200 (#373) - missing repo → 404 WITH CORS headers (#374) - path traversal (percent-encoded so fetch doesn't normalize it away) → blocked 403/404, never 200/500, with CORS - unauthenticated push advertise → 401 + WWW-Authenticate on a non-public server. Deliberately does NOT assert CORS: the WAC preHandler's 401 path doesn't set the git CORS headers today — that gap is now tracked as #548; the test asserts what is true. - end-to-end push: real `git push HEAD:main` via async spawn → exit 0 → pushed file served as a static resource (receive-pack POST through http-backend + updateInstead extraction). The push MUST use async spawn — spawnSync blocks the event loop and the in-process server can never respond (deadlock). Also fixes a latent cross-test pollution bug in test/git-auto-init.test.js: the unauthenticated-ACL test boots a second server with root './test-data-git-auto-init-authcheck', and createServer({ root }) mutates process.env.DATA_ROOT globally without restoring it. Every test after it silently ran against the WRONG data root — handleGit auto-inited public/apps/penny inside the authcheck dir, the suite cleanup missed it, and the orphaned directory kept reappearing after every full test run. Snapshot/restore the env in the test's finally block (same pollution class as the #543 review finding). The authcheck dir no longer survives a run. Full suite: 934/934 passing. Closes #375.
… auto-init suite (#375) (#549) src/handlers/git.js had zero automated coverage — #371 (CORS) and #373 (multi-slash) were verified by hand. New test/git-handler.test.js pins the HTTP contract with 8 cases: - OPTIONS preflight → 200 + canonical git CORS headers (#371) - single/double/triple-slash info/refs advertise → 200 (#373) - missing repo → 404 WITH CORS headers (#374) - path traversal (percent-encoded so fetch doesn't normalize it away) → blocked 403/404, never 200/500, with CORS - unauthenticated push advertise → 401 + WWW-Authenticate on a non-public server. Deliberately does NOT assert CORS: the WAC preHandler's 401 path doesn't set the git CORS headers today — that gap is now tracked as #548; the test asserts what is true. - end-to-end push: real `git push HEAD:main` via async spawn → exit 0 → pushed file served as a static resource (receive-pack POST through http-backend + updateInstead extraction). The push MUST use async spawn — spawnSync blocks the event loop and the in-process server can never respond (deadlock). Also fixes a latent cross-test pollution bug in test/git-auto-init.test.js: the unauthenticated-ACL test boots a second server with root './test-data-git-auto-init-authcheck', and createServer({ root }) mutates process.env.DATA_ROOT globally without restoring it. Every test after it silently ran against the WRONG data root — handleGit auto-inited public/apps/penny inside the authcheck dir, the suite cleanup missed it, and the orphaned directory kept reappearing after every full test run. Snapshot/restore the env in the test's finally block (same pollution class as the #543 review finding). The authcheck dir no longer survives a run. Full suite: 934/934 passing. Closes #375.
| Back | FazBrowse Home | New Git URL |
Closes #474.
Bug
src/server.js:193 hard-coded Fastify's bodyLimit to 10 MiB. Any caller wanting to accept larger git push payloads — e.g. an established app repo with several MB of history — got 413, with no escape hatch. Reporter hit it trying to install solid-chat/app (~30 MB pack from 3 years of history) on a jspod.
Fix
Make bodyLimit configurable across all four config surfaces JSS already supports:
String parsing reuses the existing config.parseSize helper (already used for defaultQuota). createServer normalises with a small ternary so any caller — number or string — gets the same bytes going into fastifyOptions.bodyLimit.
What's deliberately not in this PR
Tests
New test/body-limit.test.js — 3 cases:
The default (10 MiB) is implicitly covered by every other test in the suite continuing to pass.
Full suite: 920/920 passing (917 + 3 new).
Refs