-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: detect requests without body correctly #5290
Conversation
📊 Benchmark resultsComparing with fd7db45 Package size: 247 MB(no change)
|
if a request goes through an edge function the content-length header will not be set and instead the transfer-enconding is set to chunked.
src/lib/functions/server.mjs
Outdated
@@ -41,6 +41,13 @@ const buildClientContext = function (headers) { | |||
} | |||
} | |||
|
|||
const hasBody = (req) => | |||
// copied from is-type package | |||
(req.header('transfer-encoding') !== undefined || !Number.isNaN(req.header('content-length'))) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why're we specifically checking for the NaN
value here?
In the type-is package the global isNaN
function is used which behaves differently from the Number.isNaN
function since the global isNaN
is true for any non-numeric value (strings, undefined, null etc.) while the Number.isNaN
is only true for the NaN
value & nothing else.
Ref: https://github.com/jshttp/type-is/blob/7d19b7aab1ad671f59ba157ae0640cd4b1302ca5/index.js#L92-L95
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, I wasn't aware of that and I just changed it because some eslint rule recommended it.
I changed it to the global version
if a request goes through an edge function the
content-length
header will be removed and instead thetransfer-enconding
is set to chunked. Our detection in the functions (non-edge) server was detecting these passthrough requests as requests without body, because of the missingcontent-length
.content-length
should be ignored anyways whentransfer-encoding
is used according to the HTTP/1.1 spec.Fixes #5030
This also changes
request.get(...)
andrequest.headers[...]
torequest.header(...)
for readability.