From a0957177fe0b49ce8532dd26f2446300dcb38e0b Mon Sep 17 00:00:00 2001 From: ozpool Date: Thu, 14 May 2026 11:39:42 +0530 Subject: [PATCH] rpc: count error responses against batch response size limit handleBatch only accumulated len(resp.Result) when evaluating the configured batchResponseMaxSize. Error responses leave Result nil and put their payload on resp.Error, so a batch returning many errors - especially ones carrying large error.data via rpc.DataError - could exceed the limit without ever tripping the response-too-large guard. Introduce a responseSize helper that returns the marshalled size of resp.Error when present, so error payloads count toward the cap on the same footing as successful results. The helper falls back to the error message length on the (theoretically impossible) case where marshalling jsonError fails, so the counter still makes progress. Adds TestServerBatchResponseSizeLimitErrors which configures a 50-byte cap, issues a batch of five test_returnError calls, and asserts that only the first error is delivered while the remainder return code -32003 (errcodeResponseTooLarge). The test fails on master without the fix and passes with it. Fixes #33814 --- rpc/handler.go | 30 +++++++++++++++++++++++++++- rpc/server_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) 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()