From 5d7e18539e32cb4f1aafab8e977e28a7cd34da9c Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Tue, 31 Jul 2018 02:16:14 -0700 Subject: [PATCH] rpc: make HTTP RPC timeouts configurable, raise defaults (#17240) * rpc: Make HTTP server timeout values configurable * rpc: Remove flags for setting HTTP Timeouts, configuring via .toml is sufficient. * rpc: Replace separate constants with a single default struct. * rpc: Update HTTP Server Read and Write Timeouts to 30s. * rpc: Remove redundant NewDefaultHTTPTimeouts function. * rpc: document HTTPTimeouts. * rpc: sanitize timeout values for library use --- cmd/clef/main.go | 2 +- node/api.go | 2 +- node/config.go | 5 +++++ node/defaults.go | 2 ++ node/node.go | 6 +++--- rpc/endpoints.go | 4 ++-- rpc/http.go | 56 ++++++++++++++++++++++++++++++++++++++++++++---- 7 files changed, 66 insertions(+), 11 deletions(-) diff --git a/cmd/clef/main.go b/cmd/clef/main.go index 348bcb22f6..85704754de 100644 --- a/cmd/clef/main.go +++ b/cmd/clef/main.go @@ -415,7 +415,7 @@ func signer(c *cli.Context) error { // start http server httpEndpoint := fmt.Sprintf("%s:%d", c.String(utils.RPCListenAddrFlag.Name), c.Int(rpcPortFlag.Name)) - listener, _, err := rpc.StartHTTPEndpoint(httpEndpoint, rpcAPI, []string{"account"}, cors, vhosts) + listener, _, err := rpc.StartHTTPEndpoint(httpEndpoint, rpcAPI, []string{"account"}, cors, vhosts, rpc.DefaultHTTPTimeouts) if err != nil { utils.Fatalf("Could not start RPC api: %v", err) } diff --git a/node/api.go b/node/api.go index 117b76b6d8..f44c991539 100644 --- a/node/api.go +++ b/node/api.go @@ -157,7 +157,7 @@ func (api *PrivateAdminAPI) StartRPC(host *string, port *int, cors *string, apis } } - if err := api.node.startHTTP(fmt.Sprintf("%s:%d", *host, *port), api.node.rpcAPIs, modules, allowedOrigins, allowedVHosts); err != nil { + if err := api.node.startHTTP(fmt.Sprintf("%s:%d", *host, *port), api.node.rpcAPIs, modules, allowedOrigins, allowedVHosts, api.node.config.HTTPTimeouts); err != nil { return false, err } return true, nil diff --git a/node/config.go b/node/config.go index 1b75baaeb2..a4d5920a8b 100644 --- a/node/config.go +++ b/node/config.go @@ -33,6 +33,7 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/p2p/discover" + "github.com/ethereum/go-ethereum/rpc" ) const ( @@ -119,6 +120,10 @@ type Config struct { // exposed. HTTPModules []string `toml:",omitempty"` + // HTTPTimeouts allows for customization of the timeout values used by the HTTP RPC + // interface. + HTTPTimeouts rpc.HTTPTimeouts + // WSHost is the host interface on which to start the websocket RPC server. If // this field is empty, no websocket API endpoint will be started. WSHost string `toml:",omitempty"` diff --git a/node/defaults.go b/node/defaults.go index 8875605807..c1376dba0f 100644 --- a/node/defaults.go +++ b/node/defaults.go @@ -24,6 +24,7 @@ import ( "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/p2p/nat" + "github.com/ethereum/go-ethereum/rpc" ) const ( @@ -39,6 +40,7 @@ var DefaultConfig = Config{ HTTPPort: DefaultHTTPPort, HTTPModules: []string{"net", "web3"}, HTTPVirtualHosts: []string{"localhost"}, + HTTPTimeouts: rpc.DefaultHTTPTimeouts, WSPort: DefaultWSPort, WSModules: []string{"net", "web3"}, P2P: p2p.Config{ diff --git a/node/node.go b/node/node.go index 7cc11b781a..ada3837217 100644 --- a/node/node.go +++ b/node/node.go @@ -263,7 +263,7 @@ func (n *Node) startRPC(services map[reflect.Type]Service) error { n.stopInProc() return err } - if err := n.startHTTP(n.httpEndpoint, apis, n.config.HTTPModules, n.config.HTTPCors, n.config.HTTPVirtualHosts); err != nil { + if err := n.startHTTP(n.httpEndpoint, apis, n.config.HTTPModules, n.config.HTTPCors, n.config.HTTPVirtualHosts, n.config.HTTPTimeouts); err != nil { n.stopIPC() n.stopInProc() return err @@ -331,12 +331,12 @@ func (n *Node) stopIPC() { } // startHTTP initializes and starts the HTTP RPC endpoint. -func (n *Node) startHTTP(endpoint string, apis []rpc.API, modules []string, cors []string, vhosts []string) error { +func (n *Node) startHTTP(endpoint string, apis []rpc.API, modules []string, cors []string, vhosts []string, timeouts rpc.HTTPTimeouts) error { // Short circuit if the HTTP endpoint isn't being exposed if endpoint == "" { return nil } - listener, handler, err := rpc.StartHTTPEndpoint(endpoint, apis, modules, cors, vhosts) + listener, handler, err := rpc.StartHTTPEndpoint(endpoint, apis, modules, cors, vhosts, timeouts) if err != nil { return err } diff --git a/rpc/endpoints.go b/rpc/endpoints.go index 692c62d3a4..8ca6d4eb0c 100644 --- a/rpc/endpoints.go +++ b/rpc/endpoints.go @@ -23,7 +23,7 @@ import ( ) // StartHTTPEndpoint starts the HTTP RPC endpoint, configured with cors/vhosts/modules -func StartHTTPEndpoint(endpoint string, apis []API, modules []string, cors []string, vhosts []string) (net.Listener, *Server, error) { +func StartHTTPEndpoint(endpoint string, apis []API, modules []string, cors []string, vhosts []string, timeouts HTTPTimeouts) (net.Listener, *Server, error) { // Generate the whitelist based on the allowed modules whitelist := make(map[string]bool) for _, module := range modules { @@ -47,7 +47,7 @@ func StartHTTPEndpoint(endpoint string, apis []API, modules []string, cors []str if listener, err = net.Listen("tcp", endpoint); err != nil { return nil, nil, err } - go NewHTTPServer(cors, vhosts, handler).Serve(listener) + go NewHTTPServer(cors, vhosts, timeouts, handler).Serve(listener) return listener, handler, err } diff --git a/rpc/http.go b/rpc/http.go index 6388d68961..f3bd1f29c5 100644 --- a/rpc/http.go +++ b/rpc/http.go @@ -31,6 +31,7 @@ import ( "sync" "time" + "github.com/ethereum/go-ethereum/log" "github.com/rs/cors" ) @@ -66,6 +67,38 @@ func (hc *httpConn) Close() error { return nil } +// HTTPTimeouts represents the configuration params for the HTTP RPC server. +type HTTPTimeouts struct { + // ReadTimeout is the maximum duration for reading the entire + // request, including the body. + // + // Because ReadTimeout does not let Handlers make per-request + // decisions on each request body's acceptable deadline or + // upload rate, most users will prefer to use + // ReadHeaderTimeout. It is valid to use them both. + ReadTimeout time.Duration + + // WriteTimeout is the maximum duration before timing out + // writes of the response. It is reset whenever a new + // request's header is read. Like ReadTimeout, it does not + // let Handlers make decisions on a per-request basis. + WriteTimeout time.Duration + + // IdleTimeout is the maximum amount of time to wait for the + // next request when keep-alives are enabled. If IdleTimeout + // is zero, the value of ReadTimeout is used. If both are + // zero, ReadHeaderTimeout is used. + IdleTimeout time.Duration +} + +// DefaultHTTPTimeouts represents the default timeout values used if further +// configuration is not provided. +var DefaultHTTPTimeouts = HTTPTimeouts{ + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 120 * time.Second, +} + // DialHTTPWithClient creates a new RPC client that connects to an RPC server over HTTP // using the provided HTTP Client. func DialHTTPWithClient(endpoint string, client *http.Client) (*Client, error) { @@ -161,15 +194,30 @@ func (t *httpReadWriteNopCloser) Close() error { // NewHTTPServer creates a new HTTP RPC server around an API provider. // // Deprecated: Server implements http.Handler -func NewHTTPServer(cors []string, vhosts []string, srv *Server) *http.Server { +func NewHTTPServer(cors []string, vhosts []string, timeouts HTTPTimeouts, srv *Server) *http.Server { // Wrap the CORS-handler within a host-handler handler := newCorsHandler(srv, cors) handler = newVHostHandler(vhosts, handler) + + // Make sure timeout values are meaningful + if timeouts.ReadTimeout < time.Second { + log.Warn("Sanitizing invalid HTTP read timeout", "provided", timeouts.ReadTimeout, "updated", DefaultHTTPTimeouts.ReadTimeout) + timeouts.ReadTimeout = DefaultHTTPTimeouts.ReadTimeout + } + if timeouts.WriteTimeout < time.Second { + log.Warn("Sanitizing invalid HTTP write timeout", "provided", timeouts.WriteTimeout, "updated", DefaultHTTPTimeouts.WriteTimeout) + timeouts.WriteTimeout = DefaultHTTPTimeouts.WriteTimeout + } + if timeouts.IdleTimeout < time.Second { + log.Warn("Sanitizing invalid HTTP idle timeout", "provided", timeouts.IdleTimeout, "updated", DefaultHTTPTimeouts.IdleTimeout) + timeouts.IdleTimeout = DefaultHTTPTimeouts.IdleTimeout + } + // Bundle and start the HTTP server return &http.Server{ Handler: handler, - ReadTimeout: 5 * time.Second, - WriteTimeout: 10 * time.Second, - IdleTimeout: 120 * time.Second, + ReadTimeout: timeouts.ReadTimeout, + WriteTimeout: timeouts.WriteTimeout, + IdleTimeout: timeouts.IdleTimeout, } }