Repository navigation
Response body ends prematurely #803
Description
Activity
@dnlup would you mind taking a look and maybe making a test?
How does Node.js handle this case?
It just works. This might be a wasm thing.
Are we talking about something greater than
Number.MAX_SAFE_INTEGER?Are we talking about something greater than
Number.MAX_SAFE_INTEGER?No, I have a case where I'm unable to receive anything larger than ~2^31. I'm not sure if it's undici at the moment but I think it's good idea to have a test where we transfer more than 2^32 bytes.
Reacted by dnlupCould also be a problem with Node 16 http1/http2 server. I'm seeing the issue in a reverse proxy in production.
Inspecting
llhttpI see content-length is an unsigned int of 64 bits. That should exclude some issues in the parser native side.Reacted by Robert NagyI think the problem is with Node. It happens only if you pass the entire buffer to
res.write. Undici errors with messageThe other side closed.I used Node 16 to test.
Something is weird. undici always prematurely ends response at 1605405800 bytes without error. While curl properly reads all 5 GB.
@mcollina we should probably fix this before release.
14 remaining items
Sounds like
wasm2watWhat's the best way to view the contents of the wasm file?
Maybe https://webassembly.studio?
I know what's going on. Could you find
s_n_llhttp__internal__n_consume_content_lengthreplacesize_ts withuint64_ts right after it and try recompiling and running the test?Reacted by Robert Nagy and Matteo CollinaThat fixed it!
Alright, working on the real fix. Thanks for trying it.
- added a commit that references this issue
on May 12, 2021 Fix: nodejs/llparse#44 .
@ronag could you try
llhttp.cfrom the attachment and see if it still works?FWIW, this is the file that I intend to release once llparse fix lands. Feel free to ship the wasm version built from this file (after reviewing the diff) until we'll get it pushed upstream!
- added a commit that references this issue
on May 12, 2021 Fix: nodejs/llparse#44 .
@ronag could you try
llhttp.cfrom the attachment and see if it still works?All good!
Reacted by Fedor IndutnyThank you for the help @indutny !
- No worries. Always glad to fix my own mess ups 😂…On Tue, May 11, 2021 at 23:41 Daniele Belardi ***@***.***> wrote: Thank you for the help @indutny <https://github.com/indutny> ! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#803 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2HQ632KFBEM2ZR2JGJVTTNIPI3ANCNFSM44V2Z4ZQ> .
- added a commit that references this issue
on May 12, 2021 Created a new release: https://github.com/nodejs/llhttp/releases/tag/release%2Fv6.0.2
Reacted by Matteo Collina and dnlup
Haven't had time to check but I suspect we might have a problem with body sizes larger than 31 bit.undici will complete without error before reading entire response body.