HPE_INVALID_CHUNK_SIZE but works without any issue in curl, browser and other runtimes excluding Node.js

[Vexcited]

Bug Description

Whenever I run .text() or try to read the body, I get and HPE_INVALID_CHUNK_SIZE error on Node.js on every requests made to my University's VPN domain (u-vpn.unilim.fr)

Reproducible By

const response = await fetch("https://u-vpn.unilim.fr/remote/logincheck", {
  "headers": {
    "accept": "*/*",
    "cache-control": "no-store, no-cache, must-revalidate",
    "content-type": "text/plain;charset=UTF-8",
    "if-modified-since": "Sat, 1 Jan 2000 00:00:00 GMT",
    "pragma": "no-cache",
    "Referer": "https://u-vpn.unilim.fr/remote/login?lang=en",
    "Referrer-Policy": "strict-origin-when-cross-origin"
  },
  // Fake credentials, still throws the error though
  "body": "ajax=1&username=a&realm=&credential=b",
  "method": "POST"
});

// Only happens here.
console.log(await response.text());

Expected Behavior

Every other runtimes and browsers can give me the actual text response : ret=0,redir=/remote/login?&err=sslvpn_login_permission_denied&lang=en

Environment

Ubuntu 22.04 LTS, Node v20.11.0

Additional context

I can still get the data by catching the error and doing error.cause.data but this data is truncated on long responses so not usable for me on further requests.

When I say other runtimes : Bun and Deno gives the response.

Only Node.js throws...

[metcoder95]

Seems to be similar as per https://github.com/nodejs/node/issues/47528

[Uzlopak]

When I add

    this.llhttp.llhttp_set_lenient_chunked_length(this.ptr, 1)
    this.llhttp.llhttp_set_lenient_headers(this.ptr, 1)
    this.llhttp.llhttp_set_lenient_keep_alive(this.ptr, 1)
    this.llhttp.llhttp_set_lenient_transfer_encoding(this.ptr, 1)

to https://github.com/nodejs/undici/blob/5db527a535bafd248b40bdd99a984f5c66d26513/lib/client.js#L557

it still does throw. Do I set the flags wrong?

[Uzlopak]

@bnoordhuis

Do you have any suggestion?

[mcollina]

cc @ShogunPanda

[ShogunPanda]

This has been fixed in llhttp 9, which introduces llhttp_set_lenient_spaces_after_chunk_size. Once this is set the URL above will be successfully parsed.

[mcollina]

@ShogunPanda can you send a PR over?

[ShogunPanda]

Sure, will keep you posted!

[Vexcited]

Will the fix only be available for newer versions of Node.js or I can still have the fix on previous versions by installing undici manually or something ?

[ShogunPanda]

I think it would be a semver major change on undici and thus on node. So it would be Node 22.

[Uzlopak]

I kind of disagree.

If it is a bug, it should be not a major but a minor.

But as I understand it from the other lenient options, it could be a security issue to enable this by default?! If is a security issue, we should consider it to enable it via an option flag. Then it could be still a minor.

[metcoder95]

I think it would be a semver major change on undici and thus on node. So it would be Node 22.

Curious, why do you suggest it should be a major @ShogunPanda? Are disabled by default and changes current behaviour or how so?

[ShogunPanda]

The update to llhttp 9 will enable strict parsing only (non-strict mode has been removed). Note that lenient flag are a different set of options. That's why the change is semver-major.

[metcoder95]

And llhttp 9 is the only one containing this fix? If so, I see now the recommendation; I'm not sure what would be the implications for current usage, but maybe we would like to assess how we deliver this fix.

Maybe we can deliver it with the lenient flags enabled, and with the defaults on v7? We might need to expose some flags on undici for that

[Uzlopak]

Actually if i read it correctly then llhttp 9 can be configured to behave like llhttp? If i asses correctly then llhttp 9 is not considering spaces safe but llhttp8 does, so to behave like llhttp8 you would need to set llhttp_set_lenient_spaces_after_chunk_size to true?

Regarding node integradion: when node starts we could check if insecureHttpParser flag was set using the internal nodejs functions and somehow pass it to undici? It should be somehow implemented in a way that only node can set it for the internal undici instance at startup and preventing that somebody can set it via userland code.

[Uzlopak]

Also do we consider it as a bug or as a safety precaution? I would consider curl as the gold standard. Does curl consider no/some/all of the behaviors behint the lenient flags as security issues? Does it have counter measurements in place to mitigate those security issues?

Maybe we should ask the curl maintainer for some input? Is somebody with him befriended on Twitter or so, to ask him for his opinion?

[ShogunPanda]

Sorry, the situation is slightly more complicated than that. Let me clarify in detail what are the changes.

In https://github.com/nodejs/llhttp/commit/6d04465e8c98c57a17428bf7aa54cc9e0add30ff#diff-1b020119e4d8be8161260339cac1a661547fa1c3ef93b6989c52e8f89110a6f4L383 I enabled strict mode (and remove the ability to disabled) of llhttp. This changed some behaviors like accepting \n instead of \r\n or not accepting invalid characters for token class. This got released with llhttp 9.

Also, some new leniency flags (including the one of this issue) were introduced, but they are and always been disabled by default. So, if we want to stay as close as possible on llhttp 8 we don't have to do anything.

In https://github.com/nodejs/undici/pull/2705#discussion_r1479612468 we already discussed about a separate PR which will enable ALL leniency flags with a single option (like already happens in Node) and this will fix this issue. Now, I believe updating to llhttp 9 is a semver-major (due to major change in dependency), while the new option will be semver-minor since it's disabled by default.

I hope this clarifies the issue once for all. Let me know if you need additional clarification.

[metcoder95]

Thanks, @ShogunPanda that clarifies my doubts, then the approach SGTM 👍

[ckcr4lyf]

Thanks for the issue and triage. Had the same thing when I upgraded Node.JS on an otherwise working program interacting with my ESP32.

For now I've just downgraded to Node v16 as a workaround.

[larscmagnusson]

Same problem here, but for me Im doing a request going through a NetScaler gateway.. Going directly at the source works fine. Its confirmed that Netscaler adds a space-character somewhere in the last chunk, which Node does not like and aborts the stream.

Curl works fine. Node v16 works fine, but later versions of Node does not.. Including v22.2 :(

[titanism]

Same issue here, Node v18 with undici.request:

    message: 'Response does not match the HTTP/1.1 protocol (Invalid header token)',
    name: 'HTTPParserError',
    code: 'HPE_INVALID_HEADER_TOKEN',
    data: ';: mode=block\r\n' +
      'max-age=31536000;: includeSubdomains\r\n' +
      'Strict-Transport-Security: max-age=31536000; includeSubdomains\r\n' +
      'X-XSS-Protection: 1; mode=block\r\n' +
      'X-Content-Type-Options: nosniff\r\n' +
      'Host: identity.apple.com\r\n' +
      'X-Frame-Options: SAMEORIGIN\r\n' +
      '\r\n' +
      '<?xml version="1.0" encoding="UTF-8"?>\n' +
      '<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">\n' +
      '<plist version="1.0">\n' +

[mcollina]

Somebody would need to write down a set of scripts to reproduce. Once we have that, we could take a look.

A PR to fix would also be highly appreciated.

[Vexcited]

Somebody would need to write down a set of scripts to reproduce.

The HTTP request I wrote in my issue description is still throwing an error with latest Node.js.

Uncaught TypeError: terminated
    at Fetch.onAborted (node:internal/deps/undici/undici:10815:53)
    at Fetch.emit (node:events:520:28)
    at Fetch.emit (node:domain:551:15)
    at Fetch.terminate (node:internal/deps/undici/undici:9973:14)
    at Object.onError (node:internal/deps/undici/undici:10927:38)
    at Request.onError (node:internal/deps/undici/undici:2079:31)
    at Object.errorRequest (node:internal/deps/undici/undici:1576:17) {
  [cause]: HTTPParserError: Response does not match the HTTP/1.1 protocol (Invalid character in chunk size)
      at Parser.execute (node:internal/deps/undici/undici:5724:19)
      at Parser.readMore (node:internal/deps/undici/undici:5683:16)
      at TLSSocket.<anonymous> (node:internal/deps/undici/undici:6011:18)
      at TLSSocket.emit (node:events:520:28)
      at TLSSocket.emit (node:domain:551:15)
      at emitReadable_ (node:internal/streams/readable:832:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:81:21) {
    code: 'HPE_INVALID_CHUNK_SIZE',
    data: '    \r\n' +
      'ret=0,redir=/remote/login?&err=sslvpn_login_permission_denied&lang=en\r\n' +
      '0\r\n' +
      '\r\n'
  }
}

[mcollina]

That does not include the server.

[ckcr4lyf]

@mcollina I've tried to make a reproducible example here: https://github.com/ckcr4lyf/hpe-invalid-chunk

Basically I capture the exact TCP payload that my ESP8266 returns, and mock that in server.mjs , and client.mjs just makes a request to it.

If you see on of the runs, e.g. : https://github.com/ckcr4lyf/hpe-invalid-chunk/actions/runs/10401699525 , it works for Node v14, v16 and cURL. However v18+ fails.

Hopefully this can help a bit in triaging the issue.

For ease I'm also hosting a demo of it at http://rfc5746.mywaifu.best:3399 , so you can try hitting that with an HTTP client and see what you get.

If it is of interest, here is the pcap from my ESP8266: esp8266-response.pcapng.zip

[mcollina]

@ShogunPanda @Uzlopak this is likely a problem with llhttp. It might well be that it's expected and not a bug, considering it work on node v14 and v16. Maybe there is a leniency flag that can be enabled?

[ShogunPanda]

@mcollina @Uzlopak I do confirm that by using this.llhttp.llhttp_set_lenient_spaces_after_chunk_size(this.ptr, 1) fixes the problem. Now, how should a user set it?

[metcoder95]

@ShogunPanda beat me 💨

Same question.

Also asked the same here for reference: https://github.com/nodejs/undici/issues/3386#issuecomment-2238713810