Wire --body-limit into createServer; real CONTENT_LENGTH for chunked git pushes#562
Merged
Merged
Conversation
…nked git pushes Two halves of the same user-facing bug (#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 #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 #561
There was a problem hiding this comment.
Pull request overview
This PR fixes a user-facing git push failure mode for large pushes by (1) ensuring the CLI’s --body-limit / JSS_BODY_LIMIT configuration is actually forwarded into createServer(), and (2) ensuring the Git CGI environment uses the buffered request body length (not the often-missing Content-Length header for chunked pushes).
Changes:
- Wire
config.bodyLimitinto the CLIcreateServer()call so CLI-started servers respect--body-limit/JSS_BODY_LIMIT. - Fix
src/handlers/git.jsto setCONTENT_LENGTHfromrequest.body.lengthwhen available, enabling chunked pushes to be read correctly bygit http-backend. - Add an integration test that spawns
bin/jss.js startand asserts Fastify enforces the configured body limit.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/body-limit-cli.test.js | Adds CLI-level integration coverage to ensure --body-limit / env wiring reaches Fastify. |
| src/handlers/git.js | Uses buffered body length for CONTENT_LENGTH so chunked pushes don’t present as 0 bytes to git http-backend. |
| bin/jss.js | Passes bodyLimit from loaded config into createServer() for the CLI start path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+81
to
+92
| async function stopCli() { | ||
| if (!child) return; | ||
| const c = child; | ||
| child = null; | ||
| if (c.exitCode === null && c.signalCode === null) { | ||
| 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'); | ||
| await gone; | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Addressed in b17c30a — with one correction on the mechanism: the specific race described (child exits between the exitCode check and attaching the 'exit' listener) can't occur, because both statements 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. Added a comment so the next reader doesn't re-derive this.
The spirit of the finding stands, though: the final await gone was unbounded, so a pathological unkillable child (stuck in uninterruptible I/O) would hang the suite forever. It's now capped with a 2s race — SIGKILL can't be caught, so 'exit' follows promptly in every realistic case and the bound is belt-and-braces. 3/3 passing.
…ch 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.
melvincarvalho
added a commit
that referenced
this pull request
Jun 15, 2026
Two PRs merged since 0.0.207 — completing the --body-limit / large git-push story end to end: - #562 (closes #561): wire --body-limit into createServer + real CONTENT_LENGTH for chunked git pushes. The CLI flag / JSS_BODY_LIMIT were dead wiring — bin/jss.js parsed them but never passed bodyLimit to createServer(), so jss start always used the default no matter what the operator set. Separately, git sends packs over http.postBuffer (1MiB) with Transfer-Encoding: chunked and no Content-Length, so the git CGI was told '0 bytes' — a large push could not succeed regardless of the configured limit. Both fixed. - #563 (closes #563): raise the default request body limit 10MB → 20MB — a more generous out-of-the-box cap for common workloads, still configurable via --body-limit / JSS_BODY_LIMIT. Net: large 'git push' to a pod now works out of the box, and the body-limit knob (#474) actually takes effect from the CLI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #561. Related: #559 (default bundle vs limit), #560 (auto-init leftover), #474 (the knob this revives).
Two halves of one user-facing bug — a
git pushwith a pack over 10MB could not succeed no matter what the operator configured:1.
--body-limit/JSS_BODY_LIMITwere dead wiring on the CLI pathbin/jss.js startparses them intoconfig.bodyLimit, but the hand-written options object passed tocreateServer()omittedbodyLimit— sosrc/server.jsalways fell back to the 10MiB default. Only programmaticcreateServer({ bodyLimit })ever worked (which is whytest/body-limit.test.jspassed all along). One line: passbodyLimit: config.bodyLimit.2. Chunked git pushes told the CGI "0 bytes"
git sends any pack larger than
http.postBuffer(1MiB default) withTransfer-Encoding: chunkedand no Content-Length header.src/handlers/git.jsset the CGI env from that header (CONTENT_LENGTH: ... || '0'), sogit http-backendread zero bytes from a fully-buffered 13.6MB body and receive-pack died with "the remote end hung up unexpectedly". Fixed by using the buffered body's real length (request.body.length).Without fix 2, fix 1 just moves the failure from
413to a hang-up — both are needed before raising the limit helps anyone.Tests
test/body-limit-cli.test.jsspawns the real CLI (same pattern ascli-flag-like-values.test.js) and asserts the limit reaches Fastify:--body-limit 1KB→ 2KiB PUT gets 413--body-limit 5MB→ same PUT does not get 413JSS_BODY_LIMIT=1KBenv, no flag → 413Verified the tests catch the bug: 2 of 3 fail with fix 1 reverted.
End-to-end
jss start --git --body-limit 200MB+jss install solid-chat/app=chat JavaScriptSolidServer/git— the two apps that have never been installable over HTTP — now completes 2/2 with extracted working trees serving 200 (previously: 413s, then with fix 1 alone: hang-up + an empty auto-init repo, the #560 artifact).Full suite: 961 pass, 0 fail (was 958 before the 3 new tests).
GIT_TRACE_CURLevidence and the full diagnosis trail are in #561.