Web3.js cannot handle chunked transfer encodings

[NicolasPennie]

Overview

Web3js throws an error when it processes a response from an RPC server with chunked transfer encoding: Invalid response body while trying to fetch [...]: Premature close. This happens because web3js is bundling a bad version of node-fetch. The bad version of node-fetch is 2.6.12 and later versions are also impacted. Version 2.6.7 does not have this problem.
Here is the bug in node-fetch: node-fetch/node-fetch#1219

It's critical to support chunked transfer encodings since its part of the HTTP/1.1 spec. This bug is very limiting for RPC providers such as ourselves (Helius) trying to make performance optimizations.

Web3js users impacted by this bug can workaround by installing version 2.6.7 and overriding the fetch setting of the Connection object:

    const fetch = require('node-fetch');
    const connection = new Connection(connectionString, {
        commitment: 'confirmed',
        fetch: (url, init) => fetch(url, { ...init, compress: true }),
    });

Steps to reproduce

1. Create an RPC proxy that returns data with Transfer-Encoding: chunked. Alternatively you can use Helius RPCs, but we will soon be changing it due to this issue. You're welcome to create a key for free at https://dev.helius.xyz.
2. Set web3js to 1.78.0 and run npm install.
3. Create a standard connection object (new Connection('https://rpc.helius.xyz?api-key-here'))
4. Make any RPC request.

Description of bug

All details provided in the overview.

[steveluscher]

Oof. This is awful.

1. I think we're basically just going to drop support for any version of Node prior to 18 LTS, which would fix this problem. fetch is supported natively there.
2. In general, we need to junk this transport and provide an HTTP/2 compatible transport to folks running web3.js in Node.

[NicolasPennie]

Both 1 and 2 are logical. Could we revert to the working version of node-fetch in short-term?

…

On Fri, Jul 21, 2023, 12:35 a.m. Steven Luscher ***@***.***> wrote: Oof. This is awful. 1. I think we're basically just going to drop support for any version of Node prior to 18 LTS, which would fix this problem. fetch is supported natively there. 2. In general, we need to junk this transport and provide an HTTP/2 compatible transport to folks running web3.js in Node. — Reply to this email directly, view it on GitHub <#1418 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AECOQHJLJHNR7QL4YOZUTKTXRIWLPANCNFSM6AAAAAA2SEPRY4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[steveluscher]

I'm trying to remember why we upgraded, and I have a bad feeling it was because of a security advisory.

[steveluscher]

Yep. https://vulners.com/github/GHSA-R683-J2X4-V87G

[steveluscher]

Related: #1255 (comment)

[NicolasPennie]

I'm trying to remember why we upgraded, and I have a bad feeling it was because of a security advisory.

That's unfortunate. So what's the path forward? Will web3js be enforcing a Node 18+?

[steveluscher]

Isn't the solution for RPCs to do this?

From the release notes for node-fetch@2.6.8:

For responses using chunked Transfer-Encoding without a Content-Length, this adds a check at the socket's close event to verify that the last bytes received were the final chunk bytes, i.e. 0\r\n. If not, it creates a Premature close error and sends it to response.body.destroy()

cc/ @linuskendall

[NicolasPennie]

Isn't the solution for RPCs to do this?

From the release notes for node-fetch@2.6.8:

For responses using chunked Transfer-Encoding without a Content-Length, this adds a check at the socket's close event to verify that the last bytes received were the final chunk bytes, i.e. 0\r\n. If not, it creates a Premature close error and sends it to response.body.destroy()

cc/ @linuskendall

We're looking into changes on our end to work around it, yes. But RPC providers shouldn't have to. Chunked transfer encodings are part of the HTTP 1.1 spec, which has been the standard for well over a decade now. This limits RPC providers from exploring certain latency optimizations.

[steveluscher]

But RPC providers shouldn't have to…

@linuskendall: Oh? Doesn't the spec require that the final chunk be zero length, followed by an empty line?

The chunked transfer coding is complete when a chunk with a chunk-size of zero is received, possibly followed by a trailer section, and finally terminated by an empty line.

https://datatracker.ietf.org/doc/html/rfc9112#chunked.encoding

[NicolasPennie]

@linuskendall: Oh? Doesn't the spec require that the final chunk be zero length, followed by an empty line?

Ah ok, I misread this. Let me see if we can make a change to support this on our end.

Why wouldn't this cause problems for other clients though?

[steveluscher]

Why wouldn't this cause problems for other clients though?

1. They haven't updated to the spec-compliant version of node-fetch
2. They don't use node-fetch

[NicolasPennie]

1. They haven't updated to the spec-compliant version of node-fetch 2. They don't use node-fetch

What I mean is that Postman, rust clients, golang clients, native fetch, axios, and other clients all handle our responses without any issue.

It would surprise me that only node-fetch is spec-compliant.

[steveluscher]

When you get to be my age, it becomes very difficult to feign surprise at such things.

Anyway, the next version of web3.js absolutely will not use node-fetch, none the least of which because:

1. It's a nightmare
2. We're not going to support anything older than Node 18LTS
3. Node 18+ supports fetch natively
4. The next version of web3.js is going to support HTTP/2 from Node or die trying (and we'll probably backport it)

[linuskendall]

So, two things we are seeing:

1. We are seeing the same problem as Helius

2. The response headers I have been following are standards compliant..

Now standards compliant means very little on the web ... The workaround seems to be to disable chunked encoding for http/1.1? That doesnt feel great. After reviewing this I dont think node-fetch is doing the right thing either.

[steveluscher]

Response headers, @linuskendall? The problem isn’t with the headers, it’s with the response body. The body has to end with a zero-length chunk, followed by an empty line, before the socket close, or the close is considered premature.

[arhee]

We're seeing the same issue. I was wondering if there was a timeline for a fix, so that we can implement a workaround if need be

[steveluscher]

Who here is running their app in Node 18+? What are your thoughts on upgrading to #1447?

[linuskendall]

Response headers, @linuskendall? The problem isn’t with the headers, it’s with the response body. The body has to end with a zero-length chunk, followed by an empty line, before the socket close, or the close is considered premature.

Yeah I know, I guess the point was "responses". Let me double check the responses again, but last I checked we were sending standards compliant chunks.

[linuskendall]

So @steveluscher you previously reported this:

IF Content-Length is blank
  THEN IF chunked response ends with exactly 0\r\n
    ALL GOOD
  ELSE
    FATAL with premature close

This this is the current behaviour of node-fetch. The spec says:

The chunked transfer coding is complete when a chunk with a chunk-size of zero is received, possibly followed by a trailer section, and finally terminated by an empty line.

So there are two options. Either chunk-size 0 OR trailer section (your "0" I guess right?)) followed by \r\n

Now in our responses I see:

In hex:

What i am seeing is
30 0a 0d 0a 0d

Which is basically the very last chunk is a 0 length chunk (chunk-size 0) followed by \r\n. Exactly as per spec. It looks like the 0\r\n is the penulimate chunk so the total end response is : 0\r\n<empty chunk>\r\n. It looks like node-fetch is tripping up on this type of response however I believe it is standards compliant.

[linuskendall]

Why wouldn't this cause problems for other clients though?

1. They haven't updated to the spec-compliant version of node-fetch
2. They don't use node-fetch

I don't think node-fetch is spec compliant and I think they've got it wrong :)

[steveluscher]

Oooh. What's that tool, @linuskendall?

[linuskendall]

Oooh. What's that tool, @linuskendall?

It's capture traffic via tcpdump and illustrated in wireshark :) Wireshark is great for this type of thing.

[steveluscher]

@linuskendall, can you toss me a URL that produces one of these chunked responses so that I can poke and prod at it with node-fetch?

[github-actions]

🎉 This issue has been resolved in version 1.78.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[steveluscher]

@peroxy, @linuskendall, and I found the root of this problem: node-fetch/node-fetch#1767

In short, if you use node-fetch between v2.6.8 and v2.6.12, an HTTP agent in keepalive mode, and an RPC that returns chunked responses, you will hit this bug.

Solutions include:

• Use @solana/web3.js 1.78.1+ in Node 18+ (ie. the combo that uses native fetch and not node-fetch)
• Supply httpAgent: false when creating your new Connection()
• Wait for node-fetch to fix Malformed chunked response detector creates false-positive ‘premature close’ errors when HTTP Agent involved node-fetch/node-fetch#1767

[github-actions]

Because there has been no activity on this issue for 7 days since it was closed, it has been automatically locked. Please open a new issue if it requires a follow up.