fix(http): limit buffered request bodies - #15
Merged
Conversation
Member
Author
|
@greptile review |
Member
Author
|
@greptile review |
Member
Author
|
@greptile review |
Member
Author
|
@greptile review |
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.
🔗 Linked issue
Confirmed resource-exhaustion finding from the v0.0.11 performance review.
❓ Type of change
📚 Description
ReadBodypreviously retained every externally controlled request chunk and then allocated a second contiguous buffer withBuffer.concat, without any byte limit. A 128 MiB request reached about 311 MiB peak RSS in the review benchmark.This change:
Content-Lengthbefore attaching body listeners;HTTPResult(413, "Payload Too Large"), which the AntelopeJS API implementation preserves as an HTTP 413 response;ReadBody,@RawBody, and@JSONBody;Compatibility
Normal bodies up to and including 1 MiB keep their existing
Buffer/JSON behavior. Applications intentionally buffering larger bodies must pass an explicit limit, for example@RawBody(10 * 1024 * 1024). Applications can consumecontext.rawRequestdirectly when bodies should be streamed rather than buffered. Because bodies over 1 MiB now reject by default, this is marked as a compatibility-affecting security fix.Verification
pnpm run lint— passedpnpm run build— passed; declarations exposelimit?: numberpnpm --package=@antelopejs/core dlx ajs module test .— 80 passingubuntuCI, GitGuardian, and Greptile checks — passedBenchmark
Final representative synthetic benchmark of the built code: 11 independent processes per case, 7 measured repetitions per process (77 samples), 64 KiB chunks, explicit GC before each repetition, and GNU
timefor process peak RSS.Compared with the pre-Greploop PR run (7 repetitions in one process per mode):
Compared with the original unbounded v0.0.11 review measurements:
The final run uses more independent processes than the pre-Greploop run, so small timing and startup-RSS differences should not be overinterpreted. The decisive result is that both pathological cases remain near process-baseline memory rather than scaling with the 128 MiB source size.
📝 Checklist
Greptile Summary
The PR limits buffered HTTP request bodies while preserving independent limits for multiple consumers.
Content-Lengthrejection and byte-counted streaming enforcement.ReadBody,@RawBody, and@JSONBody.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (5): Last reviewed commit: "address greptile review feedback (greplo..." | Re-trigger Greptile