http response handling improvements (#14673)

Co-authored-by: Kasey Kirkham <[email protected]>

CHANGELOG.md

@@ -66,6 +66,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
 - Updated light client consensus types. [PR](https://github.com/prysmaticlabs/prysm/pull/14652)
 - Fixed pending deposits processing on Electra.
 - Modified `ListAttestationsV2`, `GetAttesterSlashingsV2` and `GetAggregateAttestationV2` endpoints to use slot to determine fork version.
+- Improvements to HTTP response handling. [pr](https://github.com/prysmaticlabs/prysm/pull/14673)
 
 ### Deprecated
 

api/client/builder/BUILD.bazel

@@ -12,6 +12,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//api:go_default_library",
+        "//api/client:go_default_library",
         "//api/server/structs:go_default_library",
         "//config/fieldparams:go_default_library",
         "//consensus-types:go_default_library",

api/client/builder/client.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/v5/api"
+	"github.com/prysmaticlabs/prysm/v5/api/client"
 	"github.com/prysmaticlabs/prysm/v5/api/server/structs"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/blocks"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces"
@@ -176,7 +177,7 @@ func (c *Client) do(ctx context.Context, method string, path string, body io.Rea
 		err = non200Err(r)
 		return
 	}
-	res, err = io.ReadAll(r.Body)
+	res, err = io.ReadAll(io.LimitReader(r.Body, client.MaxBodySize))
 	if err != nil {
 		err = errors.Wrap(err, "error reading http response body from builder server")
 		return
@@ -358,7 +359,7 @@ func (c *Client) Status(ctx context.Context) error {
 }
 
 func non200Err(response *http.Response) error {
-	bodyBytes, err := io.ReadAll(response.Body)
+	bodyBytes, err := io.ReadAll(io.LimitReader(response.Body, client.MaxErrBodySize))
 	var errMessage ErrorMessage
 	var body string
 	if err != nil {

api/client/client.go

@@ -10,11 +10,17 @@ import (
 	"github.com/pkg/errors"
 )
 
+const (
+	MaxBodySize    int64 = 1 << 23 // 8MB default, WithMaxBodySize can override
+	MaxErrBodySize int64 = 1 << 17 // 128KB
+)
+
 // Client is a wrapper object around the HTTP client.
 type Client struct {
-	hc      *http.Client
-	baseURL *url.URL
-	token   string
+	hc          *http.Client
+	baseURL     *url.URL
+	token       string
+	maxBodySize int64
 }
 
 // NewClient constructs a new client with the provided options (ex WithTimeout).
@@ -26,8 +32,9 @@ func NewClient(host string, opts ...ClientOpt) (*Client, error) {
 		return nil, err
 	}
 	c := &Client{
-		hc:      &http.Client{},
-		baseURL: u,
+		hc:          &http.Client{},
+		baseURL:     u,
+		maxBodySize: MaxBodySize,
 	}
 	for _, o := range opts {
 		o(c)
@@ -72,7 +79,7 @@ func (c *Client) NodeURL() string {
 // Get is a generic, opinionated GET function to reduce boilerplate amongst the getters in this package.
 func (c *Client) Get(ctx context.Context, path string, opts ...ReqOption) ([]byte, error) {
 	u := c.baseURL.ResolveReference(&url.URL{Path: path})
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), http.NoBody)
 	if err != nil {
 		return nil, err
 	}
@@ -89,7 +96,7 @@ func (c *Client) Get(ctx context.Context, path string, opts ...ReqOption) ([]byt
 	if r.StatusCode != http.StatusOK {
 		return nil, Non200Err(r)
 	}
-	b, err := io.ReadAll(r.Body)
+	b, err := io.ReadAll(io.LimitReader(r.Body, c.maxBodySize))
 	if err != nil {
 		return nil, errors.Wrap(err, "error reading http response body")
 	}

api/client/errors.go

@@ -25,16 +25,16 @@ var ErrInvalidNodeVersion = errors.New("invalid node version response")
 var ErrConnectionIssue = errors.New("could not connect")
 
 // Non200Err is a function that parses an HTTP response to handle responses that are not 200 with a formatted error.
-func Non200Err(response *http.Response) error {
-	bodyBytes, err := io.ReadAll(response.Body)
+func Non200Err(r *http.Response) error {
+	b, err := io.ReadAll(io.LimitReader(r.Body, MaxErrBodySize))
 	var body string
 	if err != nil {
 		body = "(Unable to read response body.)"
 	} else {
-		body = "response body:\n" + string(bodyBytes)
+		body = "response body:\n" + string(b)
 	}
-	msg := fmt.Sprintf("code=%d, url=%s, body=%s", response.StatusCode, response.Request.URL, body)
-	switch response.StatusCode {
+	msg := fmt.Sprintf("code=%d, url=%s, body=%s", r.StatusCode, r.Request.URL, body)
+	switch r.StatusCode {
 	case http.StatusNotFound:
 		return errors.Wrap(ErrNotFound, msg)
 	default:

api/client/options.go

@@ -46,3 +46,10 @@ func WithAuthenticationToken(token string) ClientOpt {
 		c.token = token
 	}
 }
+
+// WithMaxBodySize overrides the default max body size of 8MB.
+func WithMaxBodySize(size int64) ClientOpt {
+	return func(c *Client) {
+		c.maxBodySize = size
+	}
+}

beacon-chain/sync/checkpoint/BUILD.bazel

@@ -10,6 +10,7 @@ go_library(
     importpath = "github.com/prysmaticlabs/prysm/v5/beacon-chain/sync/checkpoint",
     visibility = ["//visibility:public"],
     deps = [
+        "//api/client:go_default_library",
         "//api/client/beacon:go_default_library",
         "//beacon-chain/db:go_default_library",
         "//config/params:go_default_library",

beacon-chain/sync/checkpoint/api.go

@@ -4,11 +4,14 @@ import (
 	"context"
 
 	"github.com/pkg/errors"
+	"github.com/prysmaticlabs/prysm/v5/api/client"
 	"github.com/prysmaticlabs/prysm/v5/api/client/beacon"
 	"github.com/prysmaticlabs/prysm/v5/beacon-chain/db"
 	"github.com/prysmaticlabs/prysm/v5/config/params"
 )
 
+const stateSizeLimit int64 = 1 << 29 // 512MB
+
 // APIInitializer manages initializing the beacon node using checkpoint sync, retrieving the checkpoint state and root
 // from the remote beacon node api.
 type APIInitializer struct {
@@ -18,7 +21,7 @@ type APIInitializer struct {
 // NewAPIInitializer creates an APIInitializer, handling the set up of a beacon node api client
 // using the provided host string.
 func NewAPIInitializer(beaconNodeHost string) (*APIInitializer, error) {
-	c, err := beacon.NewClient(beaconNodeHost)
+	c, err := beacon.NewClient(beaconNodeHost, client.WithMaxBodySize(stateSizeLimit))
 	if err != nil {
 		return nil, errors.Wrapf(err, "unable to parse beacon node url or hostname - %s", beaconNodeHost)
 	}
@@ -32,10 +35,9 @@ func (dl *APIInitializer) Initialize(ctx context.Context, d db.Database) error {
 	if err == nil && origin != params.BeaconConfig().ZeroHash {
 		log.Warnf("Origin checkpoint root %#x found in db, ignoring checkpoint sync flags", origin)
 		return nil
-	} else {
-		if !errors.Is(err, db.ErrNotFound) {
-			return errors.Wrap(err, "error while checking database for origin root")
-		}
+	}
+	if err != nil && !errors.Is(err, db.ErrNotFound) {
+		return errors.Wrap(err, "error while checking database for origin root")
 	}
 	od, err := beacon.DownloadFinalizedData(ctx, dl.c)
 	if err != nil {

cmd/prysmctl/checkpointsync/download.go

@@ -42,11 +42,13 @@ var downloadCmd = &cli.Command{
 	},
 }
 
+const stateSizeLimit int64 = 1 << 29 // 512MB to accommodate future state growth
+
 func cliActionDownload(_ *cli.Context) error {
 	ctx := context.Background()
 	f := downloadFlags
 
-	opts := []client.ClientOpt{client.WithTimeout(f.Timeout)}
+	opts := []client.ClientOpt{client.WithTimeout(f.Timeout), client.WithMaxBodySize(stateSizeLimit)}
 	client, err := beacon.NewClient(downloadFlags.BeaconNodeHost, opts...)
 	if err != nil {
 		return err