diff --git a/rpc/handler.go b/rpc/handler.go index c0af162f13..01daee289f 100644 --- a/rpc/handler.go +++ b/rpc/handler.go @@ -237,7 +237,13 @@ func (h *handler) handleBatch(msgs []*jsonrpcMessage) { resp := h.handleCallMsg(cp, msg) callBuffer.pushResponse(resp) if resp != nil && h.batchResponseMaxSize != 0 { - responseBytes += len(resp.Result) + // Account for both successful results and error payloads so + // that responses carrying large error.data (e.g. revert + // reasons returned via rpc.DataError) are subject to the + // same cap as oversized successful results. Pre-fix, only + // len(resp.Result) was counted and error responses with + // large Data could push the batch well past the limit. + responseBytes += responseSize(resp) if responseBytes > h.batchResponseMaxSize { err := &internalServerError{errcodeResponseTooLarge, errMsgResponseTooLarge} callBuffer.respondWithError(cp.ctx, h.conn, err) @@ -257,6 +263,28 @@ func (h *handler) handleBatch(msgs []*jsonrpcMessage) { }) } +// responseSize returns the number of bytes that should be charged against the +// batch response size limit for a single response. Successful responses +// contribute len(resp.Result); error responses contribute the marshalled size +// of the error object (including any error.Data) so that large rpc.DataError +// payloads cannot bypass the cap. +func responseSize(resp *jsonrpcMessage) int { + if resp == nil { + return 0 + } + if resp.Error == nil { + return len(resp.Result) + } + encoded, err := json.Marshal(resp.Error) + if err != nil { + // Marshalling jsonError cannot realistically fail, but if it ever + // does, fall back to a conservative non-zero estimate so the + // counter still advances and the limit is eventually tripped. + return len(resp.Error.Message) + } + return len(encoded) +} + func (h *handler) respondWithBatchTooLarge(cp *callProc, batch []*jsonrpcMessage) { resp := errorMessage(&invalidRequestError{errMsgBatchTooLarge}) // Find the first call and add its "id" field to the error. diff --git a/rpc/server_test.go b/rpc/server_test.go index 8334d4e80d..98ef116569 100644 --- a/rpc/server_test.go +++ b/rpc/server_test.go @@ -208,6 +208,56 @@ func TestServerBatchResponseSizeLimit(t *testing.T) { } } +// TestServerBatchResponseSizeLimitErrors is the regression test for #33814: +// the batch response size counter previously only accounted for len(resp.Result), +// so error responses (which set resp.Error and leave resp.Result nil) could be +// returned without bound. Large rpc.DataError payloads in particular could blow +// past the configured limit. +// +// The testService.ReturnError method returns an Error with code 444 and +// "testError data" as Data, which marshals to roughly 60 bytes per response. +// With a 100-byte cap, the first error is accepted but every subsequent error +// must trip the response-too-large path. +func TestServerBatchResponseSizeLimitErrors(t *testing.T) { + t.Parallel() + + server := newTestServer() + defer server.Stop() + // Tight cap: a single marshalled testError is ~56 bytes, so the first + // response is accepted but every subsequent response must trip the cap. + server.SetBatchLimits(100, 50) + + client := DialInProc(server) + defer client.Close() + + var batch []BatchElem + for i := 0; i < 5; i++ { + batch = append(batch, BatchElem{ + Method: "test_returnError", + Args: []any{}, + Result: new(any), + }) + } + if err := client.BatchCall(batch); err != nil { + t.Fatal("error sending batch:", err) + } + + // The first response is the genuine testError; the rest must be the + // internal response-too-large error. + if batch[0].Error == nil { + t.Fatalf("batch elem 0 expected testError, got nil") + } + for i := 1; i < len(batch); i++ { + re, ok := batch[i].Error.(Error) + if !ok { + t.Fatalf("batch elem %d expected Error, got %v (%T)", i, batch[i].Error, batch[i].Error) + } + if re.ErrorCode() != errcodeResponseTooLarge { + t.Errorf("batch elem %d wrong error code, have %d want %d", i, re.ErrorCode(), errcodeResponseTooLarge) + } + } +} + func TestServerWebsocketReadLimit(t *testing.T) { t.Parallel()