From 432753f242335fba707592b491b88dcad9154540 Mon Sep 17 00:00:00 2001 From: Andrii Date: Fri, 25 Oct 2024 13:27:00 +0300 Subject: [PATCH 1/5] Added new flag for rest max payload size --- cmd/access/node_builder/access_node_builder.go | 17 +++++++++++++---- cmd/observer/node_builder/observer_builder.go | 17 +++++++++++++---- engine/access/rest/routes/handler.go | 3 ++- engine/access/rest/routes/http_handler.go | 12 ++++++++---- engine/access/rest/routes/router.go | 8 ++++++-- engine/access/rest/routes/test_helpers.go | 1 + engine/access/rest/routes/websocket_handler.go | 2 +- engine/access/rest/server.go | 14 +++++++++----- engine/access/rest_api_test.go | 2 +- 9 files changed, 54 insertions(+), 22 deletions(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index 1da04c85888..1db49609f11 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -217,10 +217,11 @@ func DefaultAccessNodeConfig() *AccessNodeConfig { TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly.String(), // default to ENs only for now }, RestConfig: rest.Config{ - ListenAddress: "", - WriteTimeout: rest.DefaultWriteTimeout, - ReadTimeout: rest.DefaultReadTimeout, - IdleTimeout: rest.DefaultIdleTimeout, + ListenAddress: "", + WriteTimeout: rest.DefaultWriteTimeout, + ReadTimeout: rest.DefaultReadTimeout, + IdleTimeout: rest.DefaultIdleTimeout, + MaxRequestSize: routes.DefaultMaxRequestSize, }, MaxMsgSize: grpcutils.DefaultMaxMsgSize, CompressorName: grpcutils.NoCompressor, @@ -1181,6 +1182,10 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { defaultConfig.rpcConf.RestConfig.ReadTimeout, "timeout to use when reading REST request headers") flags.DurationVar(&builder.rpcConf.RestConfig.IdleTimeout, "rest-idle-timeout", defaultConfig.rpcConf.RestConfig.IdleTimeout, "idle timeout for REST connections") + flags.Int64Var(&builder.rpcConf.RestConfig.MaxRequestSize, + "rest-max-request-size", + defaultConfig.rpcConf.RestConfig.MaxRequestSize, + "the maximum request size in bytes for payload sent over REST server") flags.StringVarP(&builder.rpcConf.CollectionAddr, "static-collection-ingress-addr", "", @@ -1496,6 +1501,10 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { return errors.New("execution-data-indexing-enabled must be set if check-payer-balance is enabled") } + if builder.rpcConf.RestConfig.MaxRequestSize < 0 { + return errors.New("rest-max-request-size must be greater than or equal to 0") + } + return nil }) } diff --git a/cmd/observer/node_builder/observer_builder.go b/cmd/observer/node_builder/observer_builder.go index 21d8211daa6..35833e6fee6 100644 --- a/cmd/observer/node_builder/observer_builder.go +++ b/cmd/observer/node_builder/observer_builder.go @@ -199,10 +199,11 @@ func DefaultObserverServiceConfig() *ObserverServiceConfig { TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly.String(), // default to ENs only for now }, RestConfig: rest.Config{ - ListenAddress: "", - WriteTimeout: rest.DefaultWriteTimeout, - ReadTimeout: rest.DefaultReadTimeout, - IdleTimeout: rest.DefaultIdleTimeout, + ListenAddress: "", + WriteTimeout: rest.DefaultWriteTimeout, + ReadTimeout: rest.DefaultReadTimeout, + IdleTimeout: rest.DefaultIdleTimeout, + MaxRequestSize: routes.DefaultMaxRequestSize, }, MaxMsgSize: grpcutils.DefaultMaxMsgSize, CompressorName: grpcutils.NoCompressor, @@ -630,6 +631,10 @@ func (builder *ObserverServiceBuilder) extraFlags() { defaultConfig.rpcConf.RestConfig.ReadTimeout, "timeout to use when reading REST request headers") flags.DurationVar(&builder.rpcConf.RestConfig.IdleTimeout, "rest-idle-timeout", defaultConfig.rpcConf.RestConfig.IdleTimeout, "idle timeout for REST connections") + flags.Int64Var(&builder.rpcConf.RestConfig.MaxRequestSize, + "rest-max-request-size", + defaultConfig.rpcConf.RestConfig.MaxRequestSize, + "the maximum request size in bytes for payload sent over REST server") flags.UintVar(&builder.rpcConf.MaxMsgSize, "rpc-max-message-size", defaultConfig.rpcConf.MaxMsgSize, @@ -864,6 +869,10 @@ func (builder *ObserverServiceBuilder) extraFlags() { } } + if builder.rpcConf.RestConfig.MaxRequestSize < 0 { + return errors.New("rest-max-request-size must be greater than or equal to 0") + } + return nil }) } diff --git a/engine/access/rest/routes/handler.go b/engine/access/rest/routes/handler.go index 2779fe32699..6c05266a8f5 100644 --- a/engine/access/rest/routes/handler.go +++ b/engine/access/rest/routes/handler.go @@ -36,12 +36,13 @@ func NewHandler( handlerFunc ApiHandlerFunc, generator models.LinkGenerator, chain flow.Chain, + maxRequestSize int64, ) *Handler { handler := &Handler{ backend: backend, apiHandlerFunc: handlerFunc, linkGenerator: generator, - HttpHandler: NewHttpHandler(logger, chain), + HttpHandler: NewHttpHandler(logger, chain, maxRequestSize), } return handler diff --git a/engine/access/rest/routes/http_handler.go b/engine/access/rest/routes/http_handler.go index f6a190ba0ad..35dbffd52fd 100644 --- a/engine/access/rest/routes/http_handler.go +++ b/engine/access/rest/routes/http_handler.go @@ -16,7 +16,7 @@ import ( "github.com/onflow/flow-go/model/flow" ) -const MaxRequestSize = 2 << 20 // 2MB +const DefaultMaxRequestSize = 2 << 20 // 2MB // HttpHandler is custom http handler implementing custom handler function. // HttpHandler function allows easier handling of errors and responses as it @@ -24,15 +24,19 @@ const MaxRequestSize = 2 << 20 // 2MB type HttpHandler struct { Logger zerolog.Logger Chain flow.Chain + + MaxRequestSize int64 } func NewHttpHandler( logger zerolog.Logger, chain flow.Chain, + maxRequestSize int64, ) *HttpHandler { return &HttpHandler{ - Logger: logger, - Chain: chain, + Logger: logger, + Chain: chain, + MaxRequestSize: maxRequestSize, } } @@ -43,7 +47,7 @@ func (h *HttpHandler) VerifyRequest(w http.ResponseWriter, r *http.Request) erro errLog := h.Logger.With().Str("request_url", r.URL.String()).Logger() // limit requested body size - r.Body = http.MaxBytesReader(w, r.Body, MaxRequestSize) + r.Body = http.MaxBytesReader(w, r.Body, h.MaxRequestSize) err := r.ParseForm() if err != nil { h.errorHandler(w, err, errLog) diff --git a/engine/access/rest/routes/router.go b/engine/access/rest/routes/router.go index 57e505d7497..092b5a84f73 100644 --- a/engine/access/rest/routes/router.go +++ b/engine/access/rest/routes/router.go @@ -46,10 +46,14 @@ func NewRouterBuilder( } // AddRestRoutes adds rest routes to the router. -func (b *RouterBuilder) AddRestRoutes(backend access.API, chain flow.Chain) *RouterBuilder { +func (b *RouterBuilder) AddRestRoutes( + backend access.API, + chain flow.Chain, + maxRequestSize int64, +) *RouterBuilder { linkGenerator := models.NewLinkGeneratorImpl(b.v1SubRouter) for _, r := range Routes { - h := NewHandler(b.logger, backend, r.Handler, linkGenerator, chain) + h := NewHandler(b.logger, backend, r.Handler, linkGenerator, chain, maxRequestSize) b.v1SubRouter. Methods(r.Method). Path(r.Pattern). diff --git a/engine/access/rest/routes/test_helpers.go b/engine/access/rest/routes/test_helpers.go index feae66f5bf9..2294b11486d 100644 --- a/engine/access/rest/routes/test_helpers.go +++ b/engine/access/rest/routes/test_helpers.go @@ -126,6 +126,7 @@ func executeRequest(req *http.Request, backend access.API) *httptest.ResponseRec ).AddRestRoutes( backend, flow.Testnet.Chain(), + DefaultMaxRequestSize, ).Build() rr := httptest.NewRecorder() diff --git a/engine/access/rest/routes/websocket_handler.go b/engine/access/rest/routes/websocket_handler.go index f2261baa76f..db2c612e0cf 100644 --- a/engine/access/rest/routes/websocket_handler.go +++ b/engine/access/rest/routes/websocket_handler.go @@ -261,7 +261,7 @@ func NewWSHandler( maxStreams: int32(stateStreamConfig.MaxGlobalStreams), defaultHeartbeatInterval: stateStreamConfig.HeartbeatInterval, activeStreamCount: atomic.NewInt32(0), - HttpHandler: NewHttpHandler(logger, chain), + HttpHandler: NewHttpHandler(logger, chain, DefaultMaxRequestSize), } return handler diff --git a/engine/access/rest/server.go b/engine/access/rest/server.go index 0d05fcd67cf..67285c58960 100644 --- a/engine/access/rest/server.go +++ b/engine/access/rest/server.go @@ -24,13 +24,17 @@ const ( // DefaultIdleTimeout is the default idle timeout for the HTTP server DefaultIdleTimeout = time.Second * 60 + + // DefaultMaxRequestSize is the default message size limit + //DefaultMaxRequestSize = 2 << 20 // 2MB ) type Config struct { - ListenAddress string - WriteTimeout time.Duration - ReadTimeout time.Duration - IdleTimeout time.Duration + ListenAddress string + WriteTimeout time.Duration + ReadTimeout time.Duration + IdleTimeout time.Duration + MaxRequestSize int64 } // NewServer returns an HTTP server initialized with the REST API handler @@ -42,7 +46,7 @@ func NewServer(serverAPI access.API, stateStreamApi state_stream.API, stateStreamConfig backend.Config, ) (*http.Server, error) { - builder := routes.NewRouterBuilder(logger, restCollector).AddRestRoutes(serverAPI, chain) + builder := routes.NewRouterBuilder(logger, restCollector).AddRestRoutes(serverAPI, chain, config.MaxRequestSize) if stateStreamApi != nil { builder.AddWsRoutes(stateStreamApi, chain, stateStreamConfig) } diff --git a/engine/access/rest_api_test.go b/engine/access/rest_api_test.go index 96c6aadf150..2f28c8bda48 100644 --- a/engine/access/rest_api_test.go +++ b/engine/access/rest_api_test.go @@ -424,7 +424,7 @@ func (suite *RestAPITestSuite) TestRequestSizeRestriction() { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() // make a request of size larger than the max permitted size - requestBytes := make([]byte, routes.MaxRequestSize+1) + requestBytes := make([]byte, routes.DefaultMaxRequestSize+1) script := restclient.ScriptsBody{ Script: string(requestBytes), } From f02fec8f0643f6b2d635b1cb3396b70727ae1b09 Mon Sep 17 00:00:00 2001 From: Andrii Date: Fri, 25 Oct 2024 13:29:07 +0300 Subject: [PATCH 2/5] Removed commented code --- engine/access/rest/server.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/engine/access/rest/server.go b/engine/access/rest/server.go index 67285c58960..0d352c04ffd 100644 --- a/engine/access/rest/server.go +++ b/engine/access/rest/server.go @@ -24,9 +24,6 @@ const ( // DefaultIdleTimeout is the default idle timeout for the HTTP server DefaultIdleTimeout = time.Second * 60 - - // DefaultMaxRequestSize is the default message size limit - //DefaultMaxRequestSize = 2 << 20 // 2MB ) type Config struct { From 503274899d9fb2b116c697d4a354e200a3b2294e Mon Sep 17 00:00:00 2001 From: Andrii Date: Mon, 4 Nov 2024 12:23:20 +0200 Subject: [PATCH 3/5] Passing same operator value for request size as for rest, added check for zero --- cmd/access/node_builder/access_node_builder.go | 2 +- cmd/observer/node_builder/observer_builder.go | 2 +- engine/access/rest/routes/router.go | 3 ++- engine/access/rest/routes/test_helpers.go | 2 +- engine/access/rest/routes/websocket_handler.go | 3 ++- engine/access/rest/server.go | 2 +- 6 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index f32077cb92c..90de121b868 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -1512,7 +1512,7 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { return errors.New("execution-data-indexing-enabled must be set if check-payer-balance is enabled") } - if builder.rpcConf.RestConfig.MaxRequestSize < 0 { + if builder.rpcConf.RestConfig.MaxRequestSize <= 0 { return errors.New("rest-max-request-size must be greater than or equal to 0") } diff --git a/cmd/observer/node_builder/observer_builder.go b/cmd/observer/node_builder/observer_builder.go index e60263b966a..49cc653cecf 100644 --- a/cmd/observer/node_builder/observer_builder.go +++ b/cmd/observer/node_builder/observer_builder.go @@ -855,7 +855,7 @@ func (builder *ObserverServiceBuilder) extraFlags() { } } - if builder.rpcConf.RestConfig.MaxRequestSize < 0 { + if builder.rpcConf.RestConfig.MaxRequestSize <= 0 { return errors.New("rest-max-request-size must be greater than or equal to 0") } diff --git a/engine/access/rest/routes/router.go b/engine/access/rest/routes/router.go index 092b5a84f73..90092c3c4c7 100644 --- a/engine/access/rest/routes/router.go +++ b/engine/access/rest/routes/router.go @@ -68,10 +68,11 @@ func (b *RouterBuilder) AddWsRoutes( stateStreamApi state_stream.API, chain flow.Chain, stateStreamConfig backend.Config, + maxRequestSize int64, ) *RouterBuilder { for _, r := range WSRoutes { - h := NewWSHandler(b.logger, stateStreamApi, r.Handler, chain, stateStreamConfig) + h := NewWSHandler(b.logger, stateStreamApi, r.Handler, chain, stateStreamConfig, maxRequestSize) b.v1SubRouter. Methods(r.Method). Path(r.Pattern). diff --git a/engine/access/rest/routes/test_helpers.go b/engine/access/rest/routes/test_helpers.go index 2294b11486d..8053bf9d356 100644 --- a/engine/access/rest/routes/test_helpers.go +++ b/engine/access/rest/routes/test_helpers.go @@ -145,7 +145,7 @@ func executeWsRequest(req *http.Request, stateStreamApi state_stream.API, respon router := NewRouterBuilder(unittest.Logger(), restCollector).AddWsRoutes( stateStreamApi, - chain, config).Build() + chain, config, DefaultMaxRequestSize).Build() router.ServeHTTP(responseRecorder, req) } diff --git a/engine/access/rest/routes/websocket_handler.go b/engine/access/rest/routes/websocket_handler.go index db2c612e0cf..5e680e6e3ed 100644 --- a/engine/access/rest/routes/websocket_handler.go +++ b/engine/access/rest/routes/websocket_handler.go @@ -253,6 +253,7 @@ func NewWSHandler( subscribeFunc SubscribeHandlerFunc, chain flow.Chain, stateStreamConfig backend.Config, + maxRequestSize int64, ) *WSHandler { handler := &WSHandler{ subscribeFunc: subscribeFunc, @@ -261,7 +262,7 @@ func NewWSHandler( maxStreams: int32(stateStreamConfig.MaxGlobalStreams), defaultHeartbeatInterval: stateStreamConfig.HeartbeatInterval, activeStreamCount: atomic.NewInt32(0), - HttpHandler: NewHttpHandler(logger, chain, DefaultMaxRequestSize), + HttpHandler: NewHttpHandler(logger, chain, maxRequestSize), } return handler diff --git a/engine/access/rest/server.go b/engine/access/rest/server.go index 0d352c04ffd..a33e2e24e58 100644 --- a/engine/access/rest/server.go +++ b/engine/access/rest/server.go @@ -45,7 +45,7 @@ func NewServer(serverAPI access.API, ) (*http.Server, error) { builder := routes.NewRouterBuilder(logger, restCollector).AddRestRoutes(serverAPI, chain, config.MaxRequestSize) if stateStreamApi != nil { - builder.AddWsRoutes(stateStreamApi, chain, stateStreamConfig) + builder.AddWsRoutes(stateStreamApi, chain, stateStreamConfig, config.MaxRequestSize) } c := cors.New(cors.Options{ From dd2ec96138ecbf45c0808eaefa7cfb8f652e2c8d Mon Sep 17 00:00:00 2001 From: Andrii Diachuk Date: Tue, 5 Nov 2024 11:36:32 +0200 Subject: [PATCH 4/5] Update cmd/access/node_builder/access_node_builder.go Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> --- cmd/access/node_builder/access_node_builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index d5f76c13512..5c02be4ae2c 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -1514,7 +1514,7 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { } if builder.rpcConf.RestConfig.MaxRequestSize <= 0 { - return errors.New("rest-max-request-size must be greater than or equal to 0") + return errors.New("rest-max-request-size must be greater than 0") } return nil From c52d565ac8701d5e3c7e21c7a0400a97b1452db3 Mon Sep 17 00:00:00 2001 From: Andrii Diachuk Date: Tue, 5 Nov 2024 11:36:40 +0200 Subject: [PATCH 5/5] Update cmd/observer/node_builder/observer_builder.go Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> --- cmd/observer/node_builder/observer_builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/observer/node_builder/observer_builder.go b/cmd/observer/node_builder/observer_builder.go index 2a5d0571780..9bf08313a48 100644 --- a/cmd/observer/node_builder/observer_builder.go +++ b/cmd/observer/node_builder/observer_builder.go @@ -857,7 +857,7 @@ func (builder *ObserverServiceBuilder) extraFlags() { } if builder.rpcConf.RestConfig.MaxRequestSize <= 0 { - return errors.New("rest-max-request-size must be greater than or equal to 0") + return errors.New("rest-max-request-size must be greater than 0") } return nil