Skip to content

rpc: increase max request content length#22012

Closed
martinboehm wants to merge 1 commit intoethereum:masterfrom
martinboehm:max-request-content-length
Closed

rpc: increase max request content length#22012
martinboehm wants to merge 1 commit intoethereum:masterfrom
martinboehm:max-request-content-length

Conversation

@martinboehm
Copy link
Copy Markdown

In the ropsten network, there are multiple blocks with the size over 3MB (height 599275 - 3.35MB, 599281 - 6.52MB). The current max request content length limit of 5MB does not allow to fetch these blocks using the eth_getBlock API call. With the overhead of the hex encoding in the rpc protocol, the maximum block size possible to be fetched is only about 2.5MB.

Fixed by extending the max request content length size to 20MB - with some buffer room, as the working limit would be about 14-15MB.

In the ropsten network, there are multiple blocks with size over 3MB
(height 599275 - 3.35MB, 599281 - 6.52MB). The current limit of 5MB
does not allow to fetch these blocks using the eth_getBlock API call.
With the overhead of the hex encoding in the rpc protocol, the maximum
block size possible to be fetched is about 2.5MB.
Comment thread rpc/http.go

const (
maxRequestContentLength = 1024 * 1024 * 5
maxRequestContentLength = 1024 * 1024 * 20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps maxRequestContentLength size should not only be increased, but also be made to be configurable like parity-ethereum. As to what concrete value should it increase to, maybe DDOS attack also needs to be considered.

@fjl fjl added this to the 1.10.0 milestone Dec 15, 2020
@fjl
Copy link
Copy Markdown
Contributor

fjl commented Jan 25, 2021

I don't understand why we need to increase the maximum request length to allow for very large responses. I just tested this with the client available in package rpc and it seems to have no problem dealing with large responses sent by the server:

// This checks that maxRequestContentLength is not applied to the response of a request.
func TestHTTPRespBodyLimit(t *testing.T) {
	const respLength = maxRequestContentLength * 3

	s := NewServer()
	s.RegisterName("test", largeRespService{respLength})
	ts := httptest.NewServer(s)
	defer ts.Close()

	c, err := DialHTTP(ts.URL)
	if err != nil {
		t.Fatal(err)
	}
	defer c.Close()

	var r string
	if err := c.Call(&r, "test_largeResp"); err != nil {
		t.Fatal(err)
	}
	if len(r) != respLength {
		t.Fatalf("response has wrong length %d, want %d", len(r), respLength)
	}
}

type largeRespService struct {
	length int
}

func (x largeRespService) LargeResp() string {
	return string(make([]byte, x.length))
}

@fjl
Copy link
Copy Markdown
Contributor

fjl commented Jan 25, 2021

@martinboehm can you provide more information about your issue? It would be nice to know under which circumstances you are hitting this limit. Which client library are you using to access ropsten blocks?

@fjl fjl removed the status:triage label Jan 25, 2021
@martinboehm
Copy link
Copy Markdown
Author

@fjl Hi, we are using websocket connection, not HTTP. The issue is happening in the blockchain indexer and simple explorer https://github.com/trezor/blockbook on initial indexing of the blockchain.
The problem in the code happens here https://github.com/trezor/blockbook/blob/d0a1cb29f67b6afa9f70e6960cd8098fef9979f0/bchain/coins/eth/ethrpc.go#L476.
The issue description including the crude workaround of using sed before compilation is here trezor/blockbook#490.

@fjl
Copy link
Copy Markdown
Contributor

fjl commented Jan 27, 2021

Thanks! So it looks like the issue is with this line: https://github.com/ethereum/go-ethereum/blob/master/rpc/websocket.go#L242. I'm OK with increasing this limit independently of the HTTP request body limit, will look into it.

@fjl fjl self-assigned this Feb 16, 2021
@fjl
Copy link
Copy Markdown
Contributor

fjl commented Feb 26, 2021

Replaced by #22385

@fjl fjl closed this Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants