-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
[linux-armv7l] http.request() fails with HPE_INVALID_CONSTANT error #37053
Comments
Any progress on this? Did someone manage to reproduce it? :) |
I could reproduce this but I'm not sure whether the server response is okay. |
I did look at the other issues, but it seemed to me like those were unrelated. I'm also not sure if the server's response is 100% spec-conform, but since Node handles it without hiccup on all other platforms, and other tools even handle it on this platform, I think this should be fixed. This issue isn't uncommon either, I've encountered it with a lot of different servers as well :) |
Can you reproduce the issue on Node.js 12? And if so, is the error still raised with the |
@lpinca I can reproduce it with Node.js 12.21.0, yes. I tried to use the flag with Node 14 as well, but from the documentation it looks like it isn't fully supported anymore: The Node.js 14 and 15 docs list |
Yes the legacy http parser has been removed in Node.js 13 in favor of llhttp. cc: @indutny |
So should I open an issue in NodeJS/llhttp? Or is this still the right place for it? 🤔 |
I think it is ok. The strange thing is that the error is not raised on x64 when using the llhttp parser. |
Nice.
Just to be clear, I'd prefer if the error wouldn't be raised on armv7 either, instead of on armv7 and x64 :P Is there anything I could do to help fix/debug this? :) |
@lpinca @Chaphasilor im running into the same issue, also only on armv7, not on x64 |
@simonhbw I didn't open another issue as I was told it belonged here :) I'll try to provide a more reliable example for reproducing the issue, that doesn't rely on an external server. Hopefully this will help people fix the issue... |
@simonhbw @Chaphasilor feel free to open a new issue on nodejs/llhttp. |
Should be fixed by nodejs/llparse#44 . |
Here's the PR for Node: #38665 |
Fix: #37053 See: nodejs/llparse#44 PR-URL: #38665 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Daniele Belardi <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
@indutny just wanted to let you know that your commit went live in Node.js 16.3.0 today and so far it seems to be working! :D Waiting for it to arrive in Node 14 because that's what I'm mostly using right now and also because the |
What steps will reproduce the bug?
Run the following:
How often does it reproduce? Is there a required condition?
One this platform, always.
What is the expected behavior?
No error, the response should be parsed just fine. I know the responding web server might be (at least partially) at fault, but the issue does not appear on win32 (Windows 10) or in Linux/WSL1 (openSUSE & Debian) on x64, so this probably isn't desired behavior.
For what it's worth, both
curl
andwget
don't face this issue either.What do you see instead?
Additional information
The requested file is quite large, so if it doesn't throw an error after a few seconds, don't wait for it to finish. I wish I had another URL to shared, but right now I only have this one. Although I am very sure I've encountered this error several times before (but didn't look into it further).
Also, notice the
byteOffset
. The issue happens at a specific byte range, hence the offset. The offset isn't totally exact, but should narrow it down to a few megabytes...I really hope someone can figure out what's going on here. I'm building a download manager and it's very unfortunate that it doesn't work reliably on my server...
The text was updated successfully, but these errors were encountered: