diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index f4cf22138..ed06d6ffe 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -89,6 +89,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [types] \#4939 Block: `Round` is now `int32` - [consensus] \#4582 RoundState: `Round`, `LockedRound` & `CommitRound` are now `int32` - [consensus] \#4582 HeightVoteSet: `round` is now `int32` + - [rpc/jsonrpc/server] \#5141 Remove `WriteRPCResponseArrayHTTP` (use `WriteRPCResponseHTTP` instead) (@melekes) ### FEATURES: diff --git a/rpc/jsonrpc/server/http_json_handler.go b/rpc/jsonrpc/server/http_json_handler.go index f656c9d64..ffc244580 100644 --- a/rpc/jsonrpc/server/http_json_handler.go +++ b/rpc/jsonrpc/server/http_json_handler.go @@ -23,8 +23,9 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han return func(w http.ResponseWriter, r *http.Request) { b, err := ioutil.ReadAll(r.Body) if err != nil { - WriteRPCResponseHTTP( + WriteRPCResponseHTTPError( w, + http.StatusBadRequest, types.RPCInvalidRequestError( nil, fmt.Errorf("error reading request body: %w", err), @@ -49,8 +50,9 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han // next, try to unmarshal as a single request var request types.RPCRequest if err := json.Unmarshal(b, &request); err != nil { - WriteRPCResponseHTTP( + WriteRPCResponseHTTPError( w, + http.StatusInternalServerError, types.RPCParseError( fmt.Errorf("error unmarshalling request: %w", err), ), @@ -107,7 +109,7 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han responses = append(responses, types.NewRPCSuccessResponse(request.ID, result)) } if len(responses) > 0 { - WriteRPCResponseArrayHTTP(w, responses) + WriteRPCResponseHTTP(w, responses...) } } } diff --git a/rpc/jsonrpc/server/http_json_handler_test.go b/rpc/jsonrpc/server/http_json_handler_test.go index 260bcb11c..a5c14e59a 100644 --- a/rpc/jsonrpc/server/http_json_handler_test.go +++ b/rpc/jsonrpc/server/http_json_handler_test.go @@ -65,7 +65,7 @@ func TestRPCParams(t *testing.T) { res := rec.Result() defer res.Body.Close() // Always expecting back a JSONRPCResponse - assert.True(t, statusOK(res.StatusCode), "#%d: should always return 2XX", i) + assert.NotZero(t, res.StatusCode, "#%d: should always return code", i) blob, err := ioutil.ReadAll(res.Body) if err != nil { t.Errorf("#%d: err reading body: %v", i, err) @@ -112,7 +112,7 @@ func TestJSONRPCID(t *testing.T) { mux.ServeHTTP(rec, req) res := rec.Result() // Always expecting back a JSONRPCResponse - assert.True(t, statusOK(res.StatusCode), "#%d: should always return 2XX", i) + assert.NotZero(t, res.StatusCode, "#%d: should always return code", i) blob, err := ioutil.ReadAll(res.Body) if err != nil { t.Errorf("#%d: err reading body: %v", i, err) diff --git a/rpc/jsonrpc/server/http_server.go b/rpc/jsonrpc/server/http_server.go index b257545ad..b323d46fd 100644 --- a/rpc/jsonrpc/server/http_server.go +++ b/rpc/jsonrpc/server/http_server.go @@ -89,6 +89,9 @@ func ServeTLS( return err } +// WriteRPCResponseHTTPError marshals res as JSON and writes it to w. +// +// Panics if it can't Marshal res or write to w. func WriteRPCResponseHTTPError( w http.ResponseWriter, httpCode int, @@ -106,8 +109,18 @@ func WriteRPCResponseHTTPError( } } -func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { - jsonBytes, err := json.MarshalIndent(res, "", " ") +// WriteRPCResponseHTTP marshals res as JSON and writes it to w. +// +// Panics if it can't Marshal res or write to w. +func WriteRPCResponseHTTP(w http.ResponseWriter, res ...types.RPCResponse) { + var v interface{} + if len(res) == 1 { + v = res[0] + } else { + v = res + } + + jsonBytes, err := json.MarshalIndent(v, "", " ") if err != nil { panic(err) } @@ -118,25 +131,6 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { } } -// WriteRPCResponseArrayHTTP will do the same as WriteRPCResponseHTTP, except it -// can write arrays of responses for batched request/response interactions via -// the JSON RPC. -func WriteRPCResponseArrayHTTP(w http.ResponseWriter, res []types.RPCResponse) { - if len(res) == 1 { - WriteRPCResponseHTTP(w, res[0]) - } else { - jsonBytes, err := json.MarshalIndent(res, "", " ") - if err != nil { - panic(err) - } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(200) - if _, err := w.Write(jsonBytes); err != nil { - panic(err) - } - } -} - //----------------------------------------------------------------------------- // RecoverAndLogHandler wraps an HTTP handler, adding error logging. diff --git a/rpc/jsonrpc/server/http_server_test.go b/rpc/jsonrpc/server/http_server_test.go index 187999c44..7b5e35873 100644 --- a/rpc/jsonrpc/server/http_server_test.go +++ b/rpc/jsonrpc/server/http_server_test.go @@ -2,11 +2,13 @@ package server import ( "crypto/tls" + "errors" "fmt" "io" "io/ioutil" "net" "net/http" + "net/http/httptest" "sync" "sync/atomic" "testing" @@ -16,8 +18,13 @@ import ( "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/libs/log" + types "github.com/tendermint/tendermint/rpc/jsonrpc/types" ) +type sampleResult struct { + Value string `json:"value"` +} + func TestMaxOpenConnections(t *testing.T) { const max = 5 // max simultaneous connections @@ -93,3 +100,75 @@ func TestServeTLS(t *testing.T) { require.NoError(t, err) assert.Equal(t, []byte("some body"), body) } + +func TestWriteRPCResponseHTTP(t *testing.T) { + id := types.JSONRPCIntID(-1) + + // one argument + w := httptest.NewRecorder() + WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(id, &sampleResult{"hello"})) + resp := w.Result() + body, err := ioutil.ReadAll(resp.Body) + _ = resp.Body.Close() + require.NoError(t, err) + assert.Equal(t, 200, resp.StatusCode) + assert.Equal(t, "application/json", resp.Header.Get("Content-Type")) + assert.Equal(t, `{ + "jsonrpc": "2.0", + "id": -1, + "result": { + "value": "hello" + } +}`, string(body)) + + // multiple arguments + w = httptest.NewRecorder() + WriteRPCResponseHTTP(w, + types.NewRPCSuccessResponse(id, &sampleResult{"hello"}), + types.NewRPCSuccessResponse(id, &sampleResult{"world"})) + resp = w.Result() + body, err = ioutil.ReadAll(resp.Body) + _ = resp.Body.Close() + require.NoError(t, err) + + assert.Equal(t, 200, resp.StatusCode) + assert.Equal(t, "application/json", resp.Header.Get("Content-Type")) + assert.Equal(t, `[ + { + "jsonrpc": "2.0", + "id": -1, + "result": { + "value": "hello" + } + }, + { + "jsonrpc": "2.0", + "id": -1, + "result": { + "value": "world" + } + } +]`, string(body)) +} + +func TestWriteRPCResponseHTTPError(t *testing.T) { + w := httptest.NewRecorder() + WriteRPCResponseHTTPError(w, + http.StatusInternalServerError, + types.RPCInternalError(types.JSONRPCIntID(-1), errors.New("foo"))) + resp := w.Result() + body, err := ioutil.ReadAll(resp.Body) + _ = resp.Body.Close() + require.NoError(t, err) + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + assert.Equal(t, "application/json", resp.Header.Get("Content-Type")) + assert.Equal(t, `{ + "jsonrpc": "2.0", + "id": -1, + "error": { + "code": -32603, + "message": "Internal error", + "data": "foo" + } +}`, string(body)) +} diff --git a/rpc/jsonrpc/server/http_uri_handler.go b/rpc/jsonrpc/server/http_uri_handler.go index 7c7cd64d6..5efe0fbbe 100644 --- a/rpc/jsonrpc/server/http_uri_handler.go +++ b/rpc/jsonrpc/server/http_uri_handler.go @@ -27,7 +27,8 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit // Exception for websocket endpoints if rpcFunc.ws { return func(w http.ResponseWriter, r *http.Request) { - WriteRPCResponseHTTP(w, types.RPCMethodNotFoundError(dummyID)) + WriteRPCResponseHTTPError(w, http.StatusNotFound, + types.RPCMethodNotFoundError(dummyID)) } } @@ -40,8 +41,9 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit fnArgs, err := httpParamsToArgs(rpcFunc, r) if err != nil { - WriteRPCResponseHTTP( + WriteRPCResponseHTTPError( w, + http.StatusInternalServerError, types.RPCInvalidParamsError( dummyID, fmt.Errorf("error converting http params to arguments: %w", err), @@ -56,7 +58,8 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit logger.Debug("HTTPRestRPC", "method", r.URL.Path, "args", args, "returns", returns) result, err := unreflectResult(returns) if err != nil { - WriteRPCResponseHTTP(w, types.RPCInternalError(dummyID, err)) + WriteRPCResponseHTTPError(w, http.StatusInternalServerError, + types.RPCInternalError(dummyID, err)) return } WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(dummyID, result))