fix: give an empty request body the shape express gives it - #371
Closed
nigrosimone wants to merge 1 commit into
Closed
fix: give an empty request body the shape express gives it#371nigrosimone wants to merge 1 commit into
nigrosimone wants to merge 1 commit into
Conversation
A request with content-length: 0 returned early before any parser ran, leaving
req.body as the placeholder object every parser starts from. Express runs the
parser regardless and each one produces its own empty value, so
app.post('/upload', express.raw(), (req, res) => {
hash.update(req.body); // works on express, throws on uExpress
})
got a Buffer on express and a plain object here. Same for express.text(), where
express hands back '' and this handed back an object.
The empty case now runs the parser tail directly with an empty buffer, after the
content-type checks so a parser still ignores a request it does not handle.
Nothing is read, so this costs an allocation of a zero-length buffer. All four
tails already handled an empty buffer - json even has an explicit branch for it -
so each one produces what it should: {} for json and urlencoded, '' for text, an
empty Buffer for raw.
Verified against express 4 and express 5, which agree with each other here, and
the new test now matches both.
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.
A request with
content-length: 0returned early before any parser ran, leavingreq.bodyas the placeholder object every parser starts from. Express runs the parser regardless, and each one produces its own empty value.Same for
express.text(), where express hands back''and this handed back an object.json{}{}{}{}urlencoded{}{}{}{}text''''{}''rawBufferBuffer{}Bufferjsonandurlencodedhappened to line up because the placeholder is object-shaped;textandrawdid not.What changed
The empty case now runs the parser tail directly with an empty buffer instead of returning early, and it does so after the content-type checks, so a parser still ignores a request it does not handle. Nothing is read, so the cost is one zero-length buffer allocation on a path that previously did nothing.
All four tails already handled an empty buffer —
jsoneven has an explicitif(buf.length === 0)branch — so each one produces what it should without further changes.One thing deliberately left alone
When a parser does not claim a request, express 4 leaves
req.bodyas{}and express 5 leaves itundefined. uExpress follows express 4, which is what the suite enforces. That divergence predates this change and is not touched here — the test avoids that case so it can pass cleanly against both express versions.