Skip to content

Commit

Permalink
fix(tm2): Fix request_id mismatch at http client (#2589)
Browse files Browse the repository at this point in the history
It seems this check is not working as intended

The failing CI seems to be an existing issue not caused by these changes

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [ ] 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).
</details>

---------

Co-authored-by: Milos Zivkovic <[email protected]>
  • Loading branch information
linhpn99 and zivkovicmilos authored Jul 22, 2024
1 parent 1fbd373 commit fec2d18
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 38 deletions.
9 changes: 6 additions & 3 deletions tm2/pkg/bft/rpc/lib/client/http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ const (
protoHTTP = "http"
protoHTTPS = "https"
protoTCP = "tcp"

portHTTP = "80"
portHTTPS = "443"
)

var (
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
}
Expand Down
118 changes: 83 additions & 35 deletions tm2/pkg/bft/rpc/lib/client/http/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit fec2d18

Please sign in to comment.