Skip to content
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

Closed
Chaphasilor opened this issue Jan 24, 2021 · 16 comments
Closed

[linux-armv7l] http.request() fails with HPE_INVALID_CONSTANT error #37053

Chaphasilor opened this issue Jan 24, 2021 · 16 comments
Labels
confirmed-bug Issues with confirmed bugs. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding.

Comments

@Chaphasilor
Copy link

Chaphasilor commented Jan 24, 2021

  • Version: tested on Node v14.15.4, v15.6.0 and v14.7.x
  • Platform: Linux 5.4.51-v7l+ armv7l GNU/Linux (it's a Raspberry Pi 4 running Debian 10)
  • Subsystem: http

What steps will reproduce the bug?

Run the following:

const http = require(`http`)
const url = `http://51.77.66.14/Trilogia%20%20John%20Wick%202014-2019%20REMUX%204K%20HDR%20Latino/John%20Wick%203%20Parabellum%202019%20REMUX%204K%20HDR%20Latino.mkv`
const byteOffset = 2595000000

const req = http.request(url, {
  method: `GET`,
  headers: {
    'Range': `bytes=${byteOffset}-`,
  }
}, (res) => {
  console.log(`STATUS:`, res.statusCode);
  console.log(`HEADERS:`, res.headers);
  // res.setEncoding('hex');
  res.on('data', (chunk) => {
    // needs to be registered so that the body is parsed
  });
})

req.on('error', (err) => {
  console.error(err)
});
req.end();

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 and wget don't face this issue either.

What do you see instead?

STATUS: 206
HEADERS: {
  date: 'Sun, 24 Jan 2021 18:03:49 GMT',
  server: 'Apache/2.4.29 (Ubuntu)',
  'last-modified': 'Sun, 05 Jan 2020 20:21:31 GMT',
  etag: '"f9ac35f5c-59b6a49dd4bc5"',
  'accept-ranges': 'bytes',
  'content-length': '64426004636',
  'content-range': 'bytes 2595000000-67021004635/67021004636',
  connection: 'close',
  'content-type': 'video/x-matroska'
}
Error: Parse Error: Expected HTTP/
    at Socket.socketOnData (_http_client.js:509:22)
    at Socket.emit (events.js:315:20)
    at addChunk (internal/streams/readable.js:309:12)
    at readableAddChunk (internal/streams/readable.js:284:9)
    at Socket.Readable.push (internal/streams/readable.js:223:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:188:23) {
  bytesParsed: 335,
  code: 'HPE_INVALID_CONSTANT',
  reason: 'Expected HTTP/',
  rawPacket: <Buffer 9a 40 a4 1b 3b aa 46 a5 7e e7 09 dd 35 3f 9e 93 b6 bc 75 3e 04 ac dc e6 f8 cd 99 c4 7f 07 98 0c e7 f9 c3 2c e8 d2 20 ff d0 98 98 48 c0 ef 07 66 60 5d ... 1350 more bytes>
}

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...

@RaisinTen RaisinTen added the http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. label Jan 25, 2021
@Chaphasilor
Copy link
Author

Any progress on this? Did someone manage to reproduce it? :)

@RaisinTen
Copy link
Contributor

I could reproduce this but I'm not sure whether the server response is okay.
Did you go through the similar issues: https://github.com/nodejs/node/issues?q=is%3Aissue+http+parse+error+HPE_INVALID_CONSTANT+is%3Aclosed?
cc @nodejs/http-parser Some help please?

@Chaphasilor
Copy link
Author

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 :)

@RaisinTen RaisinTen added the confirmed-bug Issues with confirmed bugs. label Mar 11, 2021
@RaisinTen
Copy link
Contributor

cc @mscdex @lpinca
Could you please take a look at this issue?

@lpinca
Copy link
Member

lpinca commented Mar 11, 2021

Can you reproduce the issue on Node.js 12? And if so, is the error still raised with the --http-parser CLI option set to legacy?

@Chaphasilor
Copy link
Author

Chaphasilor commented Mar 12, 2021

Can you reproduce the issue on Node.js 12? And if so, is the error still raised with the --http-parser CLI option set to legacy?

@lpinca I can reproduce it with Node.js 12.21.0, yes.
However, the error is not raised using node --http-parser=legacy test.js (with Node 12).

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 --http-parser, while the Node.js 12 docs list --http-parser=library, along with some more documentation.

@lpinca
Copy link
Member

lpinca commented Mar 12, 2021

Yes the legacy http parser has been removed in Node.js 13 in favor of llhttp.

cc: @indutny

@Chaphasilor
Copy link
Author

So should I open an issue in NodeJS/llhttp?

Or is this still the right place for it? 🤔

@lpinca
Copy link
Member

lpinca commented Mar 12, 2021

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.

@Chaphasilor
Copy link
Author

I think it is ok.

Nice.

The strange thing is that the error is not raised on x64 when using the llhttp parser.

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? :)

@simonhbw
Copy link

@lpinca @Chaphasilor im running into the same issue, also only on armv7, not on x64
are there any news / fixes? did you opened an issue in nodejs/llhttp ?

@Chaphasilor
Copy link
Author

@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...

@lpinca
Copy link
Member

lpinca commented Apr 27, 2021

@simonhbw @Chaphasilor feel free to open a new issue on nodejs/llhttp.

@indutny
Copy link
Member

indutny commented May 12, 2021

Should be fixed by nodejs/llparse#44 .

indutny added a commit that referenced this issue May 13, 2021
@indutny
Copy link
Member

indutny commented May 13, 2021

Here's the PR for Node: #38665

indutny added a commit that referenced this issue May 13, 2021
danielleadams pushed a commit that referenced this issue May 31, 2021
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]>
@Chaphasilor
Copy link
Author

@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
Thanks a ton, once again!

Waiting for it to arrive in Node 14 because that's what I'm mostly using right now and also because the HPE_INVALID_CONSTANT error only appears there (probably some API change from 14 to 16 that my test code isn't yet adapted to).
Will report back after I tested it with Node 14 <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants