rpc/jsonrpc/server: merge WriteRPCResponseHTTP and WriteRPCResponseAr (#5141)

...rrayHTTP 

Closes #5135

Also, wrote a test for WriteRPCResponseHTTPError and used it with correct status codes according to https://www.jsonrpc.org/historical/json-rpc-over-http.html#response-codes

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:
 

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...)
 		}
 	}
 }

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)

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.

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))
+}

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))