You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
I know what's going on. Could you find s_n_llhttp__internal__n_consume_content_length replace size_ts with uint64_ts right after it and try recompiling and running the test?
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!
Activity
ronag commentedon May 11, 2021
@dnlup would you mind taking a look and maybe making a test?
mcollina commentedon May 11, 2021
How does Node.js handle this case?
ronag commentedon May 11, 2021
It just works. This might be a wasm thing.
dnlup commentedon May 11, 2021
Are we talking about something greater than
Number.MAX_SAFE_INTEGER?ronag commentedon May 11, 2021
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.
ronag commentedon May 11, 2021
Could also be a problem with Node 16 http1/http2 server. I'm seeing the issue in a reverse proxy in production.
dnlup commentedon May 11, 2021
Inspecting
llhttpI see content-length is an unsigned int of 64 bits. That should exclude some issues in the parser native side.dnlup commentedon May 11, 2021
I 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.
ronag commentedon May 11, 2021
Something is weird. undici always prematurely ends response at 1605405800 bytes without error. While curl properly reads all 5 GB.
ronag commentedon May 11, 2021
@mcollina @dnlup
This fails: #805
ronag commentedon May 11, 2021
@mcollina we should probably fix this before release.
14 remaining items
indutny commentedon May 12, 2021
Sounds like
wasm2watronag commentedon May 12, 2021
Maybe https://webassembly.studio?
indutny commentedon May 12, 2021
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?ronag commentedon May 12, 2021
That fixed it!
indutny commentedon May 12, 2021
Alright, working on the real fix. Thanks for trying it.
c: use proper type coercion in `consume`
indutny commentedon May 12, 2021
Fix: nodejs/llparse#44 .
@ronag could you try
llhttp.cfrom the attachment and see if it still works?llhttp.c.zip
indutny commentedon May 12, 2021
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!
c: use proper type coercion in `consume`
ronag commentedon May 12, 2021
All good!
dnlup commentedon May 12, 2021
Thank you for the help @indutny !
indutny commentedon May 12, 2021
c: use proper type coercion in `consume`
indutny commentedon May 12, 2021
Created a new release: https://github.com/nodejs/llhttp/releases/tag/release%2Fv6.0.2