From fec2d18f630b44ccc2121472aa2284cd9c8caf6f Mon Sep 17 00:00:00 2001 From: sunspirit <167175638+linhpn99@users.noreply.github.com> Date: Mon, 22 Jul 2024 16:26:58 +0700 Subject: [PATCH] fix(tm2): Fix request_id mismatch at http client (#2589) It seems this check is not working as intended The failing CI seems to be an existing issue not caused by these changes
Contributors' checklist... - [ ] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
--------- Co-authored-by: Milos Zivkovic --- tm2/pkg/bft/rpc/lib/client/http/client.go | 9 +- .../bft/rpc/lib/client/http/client_test.go | 118 ++++++++++++------ 2 files changed, 89 insertions(+), 38 deletions(-) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client.go b/tm2/pkg/bft/rpc/lib/client/http/client.go index 290310cea31..aa4fc5c5392 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client.go @@ -18,6 +18,9 @@ const ( protoHTTP = "http" protoHTTPS = "https" protoTCP = "tcp" + + portHTTP = "80" + portHTTPS = "443" ) var ( @@ -57,7 +60,7 @@ func (c *Client) SendRequest(ctx context.Context, request types.RPCRequest) (*ty } // Make sure the ID matches - if response.ID != response.ID { + if request.ID != response.ID { return nil, ErrRequestResponseIDMismatch } @@ -230,9 +233,9 @@ func parseRemoteAddr(remoteAddr string) (string, string) { if !strings.Contains(address, ":") { switch protocol { case protoHTTPS: - address += ":443" + address += ":" + portHTTPS case protoHTTP, protoTCP: - address += ":80" + address += ":" + portHTTP default: // noop } } diff --git a/tm2/pkg/bft/rpc/lib/client/http/client_test.go b/tm2/pkg/bft/rpc/lib/client/http/client_test.go index cfe69c3015c..4ccbfdc2d1e 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client_test.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client_test.go @@ -108,53 +108,101 @@ func createTestServer( func TestClient_SendRequest(t *testing.T) { t.Parallel() - var ( - request = types.RPCRequest{ - JSONRPC: "2.0", - ID: types.JSONRPCStringID("id"), - } + t.Run("valid request, response", func(t *testing.T) { + t.Parallel() - handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, http.MethodPost, r.Method) - require.Equal(t, "application/json", r.Header.Get("content-type")) + var ( + request = types.RPCRequest{ + JSONRPC: "2.0", + ID: types.JSONRPCStringID("id"), + } - // Parse the message - var req types.RPCRequest - require.NoError(t, json.NewDecoder(r.Body).Decode(&req)) - require.Equal(t, request.ID.String(), req.ID.String()) + handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + require.Equal(t, "application/json", r.Header.Get("content-type")) - // Send an empty response back - response := types.RPCResponse{ + // Parse the message + var req types.RPCRequest + require.NoError(t, json.NewDecoder(r.Body).Decode(&req)) + require.Equal(t, request.ID.String(), req.ID.String()) + + // Send an empty response back + response := types.RPCResponse{ + JSONRPC: "2.0", + ID: req.ID, + } + + // Marshal the response + marshalledResponse, err := json.Marshal(response) + require.NoError(t, err) + + _, err = w.Write(marshalledResponse) + require.NoError(t, err) + }) + + server = createTestServer(t, handler) + ) + + // Create the client + c, err := NewClient(server.URL) + require.NoError(t, err) + + ctx, cancelFn := context.WithTimeout(context.Background(), time.Second*5) + defer cancelFn() + + // Send the request + resp, err := c.SendRequest(ctx, request) + require.NoError(t, err) + + assert.Equal(t, request.ID, resp.ID) + assert.Equal(t, request.JSONRPC, resp.JSONRPC) + assert.Nil(t, resp.Result) + assert.Nil(t, resp.Error) + }) + + t.Run("response ID mismatch", func(t *testing.T) { + t.Parallel() + + var ( + request = types.RPCRequest{ JSONRPC: "2.0", - ID: req.ID, + ID: types.JSONRPCStringID("id"), } - // Marshal the response - marshalledResponse, err := json.Marshal(response) - require.NoError(t, err) + handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + require.Equal(t, "application/json", r.Header.Get("content-type")) - _, err = w.Write(marshalledResponse) - require.NoError(t, err) - }) + // Send an empty response back, + // with an invalid ID + response := types.RPCResponse{ + JSONRPC: "2.0", + ID: types.JSONRPCStringID("totally random ID"), + } - server = createTestServer(t, handler) - ) + // Marshal the response + marshalledResponse, err := json.Marshal(response) + require.NoError(t, err) - // Create the client - c, err := NewClient(server.URL) - require.NoError(t, err) + _, err = w.Write(marshalledResponse) + require.NoError(t, err) + }) - ctx, cancelFn := context.WithTimeout(context.Background(), time.Second*5) - defer cancelFn() + server = createTestServer(t, handler) + ) - // Send the request - resp, err := c.SendRequest(ctx, request) - require.NoError(t, err) + // Create the client + c, err := NewClient(server.URL) + require.NoError(t, err) - assert.Equal(t, request.ID, resp.ID) - assert.Equal(t, request.JSONRPC, resp.JSONRPC) - assert.Nil(t, resp.Result) - assert.Nil(t, resp.Error) + ctx, cancelFn := context.WithTimeout(context.Background(), time.Second*5) + defer cancelFn() + + // Send the request + resp, err := c.SendRequest(ctx, request) + assert.Nil(t, resp) + assert.ErrorIs(t, err, ErrRequestResponseIDMismatch) + }) } func TestClient_SendBatchRequest(t *testing.T) {