From aab171f8c6f5aaf071c72d0c60a7e57333be2a2c Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Mon, 30 Mar 2020 13:58:34 +0200 Subject: [PATCH] Matches the startup process of websocket server to that of http server for consistency (#8) --- cmd/geth/retesteth.go | 3 +-- graphql/service.go | 2 +- node/node.go | 16 +++++++---- rpc/endpoints.go | 62 +++++++++++++++++-------------------------- rpc/websocket.go | 7 ----- 5 files changed, 38 insertions(+), 52 deletions(-) diff --git a/cmd/geth/retesteth.go b/cmd/geth/retesteth.go index 19410d8aa5..f125fca533 100644 --- a/cmd/geth/retesteth.go +++ b/cmd/geth/retesteth.go @@ -890,13 +890,12 @@ func retesteth(ctx *cli.Context) error { srv := rpc.NewServer() - err := node.RegisterApisFromWhitelist(rpcAPI, []string{"test", "eth", "debug", "web3"}, srv) + err := node.RegisterApisFromWhitelist(rpcAPI, []string{"test", "eth", "debug", "web3"}, srv, false) if err != nil { utils.Fatalf("Could not register RPC apis: %w", err) // TODO should this be a fatal error? } handler := node.NewHTTPHandlerStack(srv, cors, vhosts) - handler = node.NewWebsocketUpgradeHandler(handler, nil) // start http server var RetestethHTTPTimeouts = rpc.HTTPTimeouts{ diff --git a/graphql/service.go b/graphql/service.go index 770db77596..2a1da808c3 100644 --- a/graphql/service.go +++ b/graphql/service.go @@ -86,7 +86,7 @@ func (s *Service) Start(server *p2p.Server) error { return nil } -func registerTimeouts(timeouts *rpc.HTTPTimeouts){ +func registerTimeouts(timeouts *rpc.HTTPTimeouts) { if timeouts.ReadTimeout < time.Second { log.Warn("Sanitizing invalid HTTP read timeout", "provided", timeouts.ReadTimeout, "updated", rpc.DefaultHTTPTimeouts.ReadTimeout) timeouts.ReadTimeout = rpc.DefaultHTTPTimeouts.ReadTimeout diff --git a/node/node.go b/node/node.go index 5f5ec33eae..325e25ba4d 100644 --- a/node/node.go +++ b/node/node.go @@ -371,7 +371,7 @@ func (n *Node) startHTTP(endpoint string, apis []rpc.API, modules []string, cors srv := rpc.NewServer() - err := RegisterApisFromWhitelist(apis, modules, srv) + err := RegisterApisFromWhitelist(apis, modules, srv, false) if err != nil { return err // TODO this should return upon failure, right? } @@ -420,7 +420,13 @@ func (n *Node) startWS(endpoint string, apis []rpc.API, modules []string, wsOrig if endpoint == "" { return nil } - listener, handler, err := rpc.StartWSEndpoint(endpoint, apis, modules, wsOrigins, exposeAll) + + srv := rpc.NewServer() + handler := srv.WebsocketHandler(wsOrigins) + + err := RegisterApisFromWhitelist(apis, modules, srv, exposeAll) + + listener, err := rpc.StartWSEndpoint(endpoint, handler) if err != nil { return err } @@ -428,7 +434,7 @@ func (n *Node) startWS(endpoint string, apis []rpc.API, modules []string, wsOrig // All listeners booted successfully n.wsEndpoint = endpoint n.wsListener = listener - n.wsHandler = handler + n.wsHandler = srv return nil } @@ -688,7 +694,7 @@ func (n *Node) apis() []rpc.API { // RegisterApisFromWhitelist checks the given modules' availability, generates a whitelist based on the allowed modules, // and then registers all of the APIs exposed by the services. -func RegisterApisFromWhitelist(apis []rpc.API, modules []string, srv *rpc.Server) error { +func RegisterApisFromWhitelist(apis []rpc.API, modules []string, srv *rpc.Server, exposeAll bool) error { if bad, available := rpc.CheckModuleAvailability(modules, apis); len(bad) > 0 { log.Error("Unavailable modules in HTTP API list", "unavailable", bad, "available", available) } @@ -699,7 +705,7 @@ func RegisterApisFromWhitelist(apis []rpc.API, modules []string, srv *rpc.Server } // Register all the APIs exposed by the services for _, api := range apis { - if whitelist[api.Namespace] || (len(whitelist) == 0 && api.Public) { + if exposeAll || whitelist[api.Namespace] || (len(whitelist) == 0 && api.Public) { if err := srv.RegisterName(api.Namespace, api.Service); err != nil { return err } diff --git a/rpc/endpoints.go b/rpc/endpoints.go index 5fe924e93d..9b70a4d923 100644 --- a/rpc/endpoints.go +++ b/rpc/endpoints.go @@ -45,7 +45,7 @@ func CheckModuleAvailability(modules []string, apis []API) (bad, available []str // StartHTTPEndpoint starts the HTTP RPC endpoint, configured with cors/vhosts/modules. func StartHTTPEndpoint(endpoint string, timeouts HTTPTimeouts, handler http.Handler) (net.Listener, error) { - // Start the HTTP listener + // start the HTTP listener var ( listener net.Listener err error @@ -53,19 +53,8 @@ func StartHTTPEndpoint(endpoint string, timeouts HTTPTimeouts, handler http.Hand if listener, err = net.Listen("tcp", endpoint); err != nil { return nil, err } - // 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 - } + + CheckTimeouts(&timeouts) // Bundle and start the HTTP server httpSrv := &http.Server{ Handler: handler, @@ -78,35 +67,18 @@ func StartHTTPEndpoint(endpoint string, timeouts HTTPTimeouts, handler http.Hand } // StartWSEndpoint starts a websocket endpoint. -func StartWSEndpoint(endpoint string, apis []API, modules []string, wsOrigins []string, exposeAll bool) (net.Listener, *Server, error) { - if bad, available := CheckModuleAvailability(modules, apis); len(bad) > 0 { - log.Error("Unavailable modules in WS API list", "unavailable", bad, "available", available) - } - // Generate the whitelist based on the allowed modules - whitelist := make(map[string]bool) - for _, module := range modules { - whitelist[module] = true - } - // Register all the APIs exposed by the services - handler := NewServer() - for _, api := range apis { - if exposeAll || whitelist[api.Namespace] || (len(whitelist) == 0 && api.Public) { - if err := handler.RegisterName(api.Namespace, api.Service); err != nil { - return nil, nil, err - } - log.Debug("WebSocket registered", "service", api.Service, "namespace", api.Namespace) - } - } - // All APIs registered, start the HTTP listener +func StartWSEndpoint(endpoint string, handler http.Handler) (net.Listener, error) { + // start the HTTP listener var ( listener net.Listener err error ) if listener, err = net.Listen("tcp", endpoint); err != nil { - return nil, nil, err + return nil, err } - go NewWSServer(wsOrigins, handler).Serve(listener) - return listener, handler, err + wsSrv := &http.Server{Handler: handler} + go wsSrv.Serve(listener) + return listener, err } // StartIPCEndpoint starts an IPC endpoint. @@ -127,3 +99,19 @@ func StartIPCEndpoint(ipcEndpoint string, apis []API) (net.Listener, *Server, er go handler.ServeListener(listener) return listener, handler, nil } + +// CheckTimeouts ensures that timeout values are meaningful +func CheckTimeouts(timeouts *HTTPTimeouts) { + 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 + } +} diff --git a/rpc/websocket.go b/rpc/websocket.go index b7ec56c6a6..6e37b8522d 100644 --- a/rpc/websocket.go +++ b/rpc/websocket.go @@ -38,13 +38,6 @@ const ( var wsBufferPool = new(sync.Pool) -// NewWSServer creates a new websocket RPC server around an API provider. -// -// Deprecated: use Server.WebsocketHandler -func NewWSServer(allowedOrigins []string, srv *Server) *http.Server { - return &http.Server{Handler: srv.WebsocketHandler(allowedOrigins)} -} - // WebsocketHandler returns a handler that serves JSON-RPC to WebSocket connections. // // allowedOrigins should be a comma-separated list of allowed origin URLs.