feat: add assertBodySize util and bodyLimit middleware#1222
Conversation
|
I need to add tests for this feature as well |
|
@pi0 do u think I should add more tests |
pi0
changed the title
assertBodySize util and bodyLimit middleware
|
Now we can add it a a Nitro v3 route rule even! |
| const bodyLen = req.headers.get("content-length"); | ||
| if (bodyLen !== null && !req.headers.has("transfer-encoding")) { | ||
| return +bodyLen <= limit; | ||
| } |
There was a problem hiding this comment.
@pi0 see #1150 (comment)
by your previous statement this code is insecure
There was a problem hiding this comment.
We should add a flag to force check when no length.
But also test which runtimes allow body with length (invalid) + transfer-encoding (if they do yes indeed we should enable flag by default)
There was a problem hiding this comment.
According to the HTTP/1.1 specification (RFC 7230, section 3.3.2):
A message must not include both a Content-Length header field and a Transfer-Encoding header field.
(we should actually fail in this case)
There was a problem hiding this comment.
but it stills allows a faked content-length
There was a problem hiding this comment.
When running in real HTTP server (Node.js server for example) it should stop reading body when content-length excceds as it violated protocol.
There was a problem hiding this comment.
(Bun and Deno also "cut off" reading body by validating content-length)
3 participants
resolves #1221