From 3764eef44c57e87a45c6ebef34154a1053730b1e Mon Sep 17 00:00:00 2001 From: devopsbo3 <69951731+devopsbo3@users.noreply.github.com> Date: Fri, 10 Nov 2023 12:27:53 -0600 Subject: [PATCH] Revert "eth, rpc: add configurable option for wsMessageSizeLimit (#27801)" This reverts commit 07bb3971330471caa87605a9af8d97ab0f097086. --- rpc/client_opt.go | 11 +------- rpc/server_test.go | 2 +- rpc/testservice_test.go | 4 --- rpc/websocket.go | 14 ++++------ rpc/websocket_test.go | 62 +---------------------------------------- 5 files changed, 8 insertions(+), 85 deletions(-) diff --git a/rpc/client_opt.go b/rpc/client_opt.go index 3fa045a9b9..5bef08cca8 100644 --- a/rpc/client_opt.go +++ b/rpc/client_opt.go @@ -34,8 +34,7 @@ type clientConfig struct { httpAuth HTTPAuth // WebSocket options - wsDialer *websocket.Dialer - wsMessageSizeLimit *int64 // wsMessageSizeLimit nil = default, 0 = no limit + wsDialer *websocket.Dialer // RPC handler options idgen func() ID @@ -67,14 +66,6 @@ func WithWebsocketDialer(dialer websocket.Dialer) ClientOption { }) } -// WithWebsocketMessageSizeLimit configures the websocket message size limit used by the RPC -// client. Passing a limit of 0 means no limit. -func WithWebsocketMessageSizeLimit(messageSizeLimit int64) ClientOption { - return optionFunc(func(cfg *clientConfig) { - cfg.wsMessageSizeLimit = &messageSizeLimit - }) -} - // WithHeader configures HTTP headers set by the RPC client. Headers set using this option // will be used for both HTTP and WebSocket connections. func WithHeader(key, value string) ClientOption { diff --git a/rpc/server_test.go b/rpc/server_test.go index 47a15b610a..5d3929dfdc 100644 --- a/rpc/server_test.go +++ b/rpc/server_test.go @@ -45,7 +45,7 @@ func TestServerRegisterName(t *testing.T) { t.Fatalf("Expected service calc to be registered") } - wantCallbacks := 14 + wantCallbacks := 13 if len(svc.callbacks) != wantCallbacks { t.Errorf("Expected %d callbacks for service 'service', got %d", wantCallbacks, len(svc.callbacks)) } diff --git a/rpc/testservice_test.go b/rpc/testservice_test.go index 7d873af667..eab67f1dd5 100644 --- a/rpc/testservice_test.go +++ b/rpc/testservice_test.go @@ -90,10 +90,6 @@ func (s *testService) EchoWithCtx(ctx context.Context, str string, i int, args * return echoResult{str, i, args} } -func (s *testService) Repeat(msg string, i int) string { - return strings.Repeat(msg, i) -} - func (s *testService) PeerInfo(ctx context.Context) PeerInfo { return PeerInfoFromContext(ctx) } diff --git a/rpc/websocket.go b/rpc/websocket.go index 538e53a31b..86cf50594c 100644 --- a/rpc/websocket.go +++ b/rpc/websocket.go @@ -38,7 +38,7 @@ const ( wsPingInterval = 30 * time.Second wsPingWriteTimeout = 5 * time.Second wsPongTimeout = 30 * time.Second - wsDefaultReadLimit = 32 * 1024 * 1024 + wsMessageSizeLimit = 32 * 1024 * 1024 ) var wsBufferPool = new(sync.Pool) @@ -60,7 +60,7 @@ func (s *Server) WebsocketHandler(allowedOrigins []string) http.Handler { log.Debug("WebSocket upgrade failed", "err", err) return } - codec := newWebsocketCodec(conn, r.Host, r.Header, wsDefaultReadLimit) + codec := newWebsocketCodec(conn, r.Host, r.Header) s.ServeCodec(codec, 0) }) } @@ -251,11 +251,7 @@ func newClientTransportWS(endpoint string, cfg *clientConfig) (reconnectFunc, er } return nil, hErr } - messageSizeLimit := int64(wsDefaultReadLimit) - if cfg.wsMessageSizeLimit != nil && *cfg.wsMessageSizeLimit >= 0 { - messageSizeLimit = *cfg.wsMessageSizeLimit - } - return newWebsocketCodec(conn, dialURL, header, messageSizeLimit), nil + return newWebsocketCodec(conn, dialURL, header), nil } return connect, nil } @@ -287,8 +283,8 @@ type websocketCodec struct { pongReceived chan struct{} } -func newWebsocketCodec(conn *websocket.Conn, host string, req http.Header, readLimit int64) ServerCodec { - conn.SetReadLimit(readLimit) +func newWebsocketCodec(conn *websocket.Conn, host string, req http.Header) ServerCodec { + conn.SetReadLimit(wsMessageSizeLimit) encode := func(v interface{}, isErrorResponse bool) error { return conn.WriteJSON(v) } diff --git a/rpc/websocket_test.go b/rpc/websocket_test.go index e4ac5c3fad..fb9357605b 100644 --- a/rpc/websocket_test.go +++ b/rpc/websocket_test.go @@ -113,66 +113,6 @@ func TestWebsocketLargeCall(t *testing.T) { } } -// This test checks whether the wsMessageSizeLimit option is obeyed. -func TestWebsocketLargeRead(t *testing.T) { - t.Parallel() - - var ( - srv = newTestServer() - httpsrv = httptest.NewServer(srv.WebsocketHandler([]string{"*"})) - wsURL = "ws:" + strings.TrimPrefix(httpsrv.URL, "http:") - ) - defer srv.Stop() - defer httpsrv.Close() - - testLimit := func(limit *int64) { - opts := []ClientOption{} - expLimit := int64(wsDefaultReadLimit) - if limit != nil && *limit >= 0 { - opts = append(opts, WithWebsocketMessageSizeLimit(*limit)) - if *limit > 0 { - expLimit = *limit // 0 means infinite - } - } - client, err := DialOptions(context.Background(), wsURL, opts...) - if err != nil { - t.Fatalf("can't dial: %v", err) - } - defer client.Close() - // Remove some bytes for json encoding overhead. - underLimit := int(expLimit - 128) - overLimit := expLimit + 1 - if expLimit == wsDefaultReadLimit { - // No point trying the full 32MB in tests. Just sanity-check that - // it's not obviously limited. - underLimit = 1024 - overLimit = -1 - } - var res string - // Check under limit - if err = client.Call(&res, "test_repeat", "A", underLimit); err != nil { - t.Fatalf("unexpected error with limit %d: %v", expLimit, err) - } - if len(res) != underLimit || strings.Count(res, "A") != underLimit { - t.Fatal("incorrect data") - } - // Check over limit - if overLimit > 0 { - err = client.Call(&res, "test_repeat", "A", expLimit+1) - if err == nil || err != websocket.ErrReadLimit { - t.Fatalf("wrong error with limit %d: %v expecting %v", expLimit, err, websocket.ErrReadLimit) - } - } - } - ptr := func(v int64) *int64 { return &v } - - testLimit(ptr(-1)) // Should be ignored (use default) - testLimit(ptr(0)) // Should be ignored (use default) - testLimit(nil) // Should be ignored (use default) - testLimit(ptr(200)) - testLimit(ptr(wsDefaultReadLimit * 2)) -} - func TestWebsocketPeerInfo(t *testing.T) { var ( s = newTestServer() @@ -266,7 +206,7 @@ func TestClientWebsocketLargeMessage(t *testing.T) { defer srv.Stop() defer httpsrv.Close() - respLength := wsDefaultReadLimit - 50 + respLength := wsMessageSizeLimit - 50 srv.RegisterName("test", largeRespService{respLength}) c, err := DialWebsocket(context.Background(), wsURL, "")