rpc: allow using a custom http client in rpc client (#3779)

fixes #2010

* allow using a custom http client in rpc client

* add tests and fix bug

* fix confusion between remote and address

* parse remote inside NewJSONRPCClientWithHTTPClient

* cleanups

* add warnings

* add changelog entry

* Update CHANGELOG_PENDING.md

Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>

CHANGELOG_PENDING.md

@@ -19,4 +19,6 @@ program](https://hackerone.com/tendermint).
 
 ### IMPROVEMENTS:
 
+- [rpc] \#2010 Add NewHTTPWithClient and NewJSONRPCClientWithHTTPClient (note these and NewHTTP, NewJSONRPCClient functions panic if remote is invalid) (@gracenoah)
+
 ### BUG FIXES:

rpc/client/httpclient.go

@@ -2,6 +2,7 @@ package client
 
 import (
 	"context"
+	"net/http"
 	"strings"
 	"sync"
 	"time"
@@ -84,8 +85,19 @@ var _ rpcClient = (*baseRPCClient)(nil)
 
 // NewHTTP takes a remote endpoint in the form <protocol>://<host>:<port> and
 // the websocket path (which always seems to be "/websocket")
+// The function panics if the provided remote is invalid.<Paste>
 func NewHTTP(remote, wsEndpoint string) *HTTP {
-	rc := rpcclient.NewJSONRPCClient(remote)
+	httpClient := rpcclient.DefaultHTTPClient(remote)
+	return NewHTTPWithClient(remote, wsEndpoint, httpClient)
+}
+
+// NewHTTPWithClient allows for setting a custom http client. See NewHTTP
+// The function panics if the provided client is nil or remote is invalid.
+func NewHTTPWithClient(remote, wsEndpoint string, client *http.Client) *HTTP {
+	if client == nil {
+		panic("nil http.Client provided")
+	}
+	rc := rpcclient.NewJSONRPCClientWithHTTPClient(remote, client)
 	cdc := rc.Codec()
 	ctypes.RegisterAmino(cdc)
 	rc.SetCodec(cdc)

rpc/client/rpc_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/tendermint/tendermint/privval"
 	"github.com/tendermint/tendermint/rpc/client"
 	ctypes "github.com/tendermint/tendermint/rpc/core/types"
+	rpcclient "github.com/tendermint/tendermint/rpc/lib/client"
 	rpctest "github.com/tendermint/tendermint/rpc/test"
 	"github.com/tendermint/tendermint/types"
 )
@@ -41,6 +42,23 @@ func GetClients() []client.Client {
 	}
 }
 
+func TestNilCustomHTTPClient(t *testing.T) {
+	require.Panics(t, func() {
+		client.NewHTTPWithClient("http://example.com", "/websocket", nil)
+	})
+	require.Panics(t, func() {
+		rpcclient.NewJSONRPCClientWithHTTPClient("http://example.com", nil)
+	})
+}
+
+func TestCustomHTTPClient(t *testing.T) {
+	remote := rpctest.GetConfig().RPC.ListenAddress
+	c := client.NewHTTPWithClient(remote, "/websocket", http.DefaultClient)
+	status, err := c.Status()
+	require.NoError(t, err)
+	require.NotNil(t, status)
+}
+
 func TestCorsEnabled(t *testing.T) {
 	origin := rpctest.GetConfig().RPC.CORSAllowedOrigins[0]
 	remote := strings.Replace(rpctest.GetConfig().RPC.ListenAddress, "tcp", "http", -1)

rpc/lib/client/http_client.go

@@ -35,11 +35,41 @@ type HTTPClient interface {
 	SetCodec(*amino.Codec)
 }
 
-// TODO: Deprecate support for IP:PORT or /path/to/socket
-func makeHTTPDialer(remoteAddr string) (string, string, func(string, string) (net.Conn, error)) {
-	// protocol to use for http operations, to support both http and https
-	clientProtocol := protoHTTP
+// protocol - client's protocol (for example, "http", "https", "wss", "ws", "tcp")
+// trimmedS - rest of the address (for example, "192.0.2.1:25", "[2001:db8::1]:80") with "/" replaced with "."
+func toClientAddrAndParse(remoteAddr string) (network string, trimmedS string, err error) {
+	protocol, address, err := parseRemoteAddr(remoteAddr)
+	if err != nil {
+		return "", "", err
+	}
 
+	// protocol to use for http operations, to support both http and https
+	var clientProtocol string
+	// default to http for unknown protocols (ex. tcp)
+	switch protocol {
+	case protoHTTP, protoHTTPS, protoWS, protoWSS:
+		clientProtocol = protocol
+	default:
+		clientProtocol = protoHTTP
+	}
+
+	// replace / with . for http requests (kvstore domain)
+	trimmedAddress := strings.Replace(address, "/", ".", -1)
+	return clientProtocol, trimmedAddress, nil
+}
+
+func toClientAddress(remoteAddr string) (string, error) {
+	clientProtocol, trimmedAddress, err := toClientAddrAndParse(remoteAddr)
+	if err != nil {
+		return "", err
+	}
+	return clientProtocol + "://" + trimmedAddress, nil
+}
+
+// network - name of the network (for example, "tcp", "unix")
+// s - rest of the address (for example, "192.0.2.1:25", "[2001:db8::1]:80")
+// TODO: Deprecate support for IP:PORT or /path/to/socket
+func parseRemoteAddr(remoteAddr string) (network string, s string, err error) {
 	parts := strings.SplitN(remoteAddr, "://", 2)
 	var protocol, address string
 	switch {
@@ -49,38 +79,44 @@ func makeHTTPDialer(remoteAddr string) (string, string, func(string, string) (ne
 	case len(parts) == 2:
 		protocol, address = parts[0], parts[1]
 	default:
-		// return a invalid message
-		msg := fmt.Sprintf("Invalid addr: %s", remoteAddr)
-		return clientProtocol, msg, func(_ string, _ string) (net.Conn, error) {
-			return nil, errors.New(msg)
-		}
+		return "", "", fmt.Errorf("invalid addr: %s", remoteAddr)
 	}
 
-	// accept http as an alias for tcp and set the client protocol
+	// accept http(s) as an alias for tcp
 	switch protocol {
 	case protoHTTP, protoHTTPS:
-		clientProtocol = protocol
 		protocol = protoTCP
-	case protoWS, protoWSS:
-		clientProtocol = protocol
 	}
 
-	// replace / with . for http requests (kvstore domain)
-	trimmedAddress := strings.Replace(address, "/", ".", -1)
-	return clientProtocol, trimmedAddress, func(proto, addr string) (net.Conn, error) {
+	return protocol, address, nil
+}
+
+func makeErrorDialer(err error) func(string, string) (net.Conn, error) {
+	return func(_ string, _ string) (net.Conn, error) {
+		return nil, err
+	}
+}
+
+func makeHTTPDialer(remoteAddr string) func(string, string) (net.Conn, error) {
+	protocol, address, err := parseRemoteAddr(remoteAddr)
+	if err != nil {
+		return makeErrorDialer(err)
+	}
+
+	return func(proto, addr string) (net.Conn, error) {
 		return net.Dial(protocol, address)
 	}
 }
 
+// DefaultHTTPClient is used to create an http client with some default parameters.
 // We overwrite the http.Client.Dial so we can do http over tcp or unix.
 // remoteAddr should be fully featured (eg. with tcp:// or unix://)
-func makeHTTPClient(remoteAddr string) (string, *http.Client) {
-	protocol, address, dialer := makeHTTPDialer(remoteAddr)
-	return protocol + "://" + address, &http.Client{
+func DefaultHTTPClient(remoteAddr string) *http.Client {
+	return &http.Client{
 		Transport: &http.Transport{
 			// Set to true to prevent GZIP-bomb DoS attacks
 			DisableCompression: true,
-			Dial:               dialer,
+			Dial:               makeHTTPDialer(remoteAddr),
 		},
 	}
 }
@@ -124,9 +160,23 @@ var _ JSONRPCCaller = (*JSONRPCRequestBatch)(nil)
 
 // NewJSONRPCClient returns a JSONRPCClient pointed at the given address.
 func NewJSONRPCClient(remote string) *JSONRPCClient {
-	address, client := makeHTTPClient(remote)
+	return NewJSONRPCClientWithHTTPClient(remote, DefaultHTTPClient(remote))
+}
+
+// NewJSONRPCClientWithHTTPClient returns a JSONRPCClient pointed at the given address using a custom http client
+// The function panics if the provided client is nil or remote is invalid.
+func NewJSONRPCClientWithHTTPClient(remote string, client *http.Client) *JSONRPCClient {
+	if client == nil {
+		panic("nil http.Client provided")
+	}
+
+	clientAddress, err := toClientAddress(remote)
+	if err != nil {
+		panic(fmt.Sprintf("invalid remote %s: %s", remote, err))
+	}
+
 	return &JSONRPCClient{
-		address: address,
+		address: clientAddress,
 		client:  client,
 		id:      types.JSONRPCStringID("jsonrpc-client-" + cmn.RandStr(8)),
 		cdc:     amino.NewCodec(),
@@ -259,11 +309,15 @@ type URIClient struct {
 	cdc     *amino.Codec
 }
 
+// The function panics if the provided remote is invalid.
 func NewURIClient(remote string) *URIClient {
-	address, client := makeHTTPClient(remote)
+	clientAddress, err := toClientAddress(remote)
+	if err != nil {
+		panic(fmt.Sprintf("invalid remote %s: %s", remote, err))
+	}
 	return &URIClient{
-		address: address,
-		client:  client,
+		address: clientAddress,
+		client:  DefaultHTTPClient(remote),
 		cdc:     amino.NewCodec(),
 	}
 }

rpc/lib/client/ws_client.go

@@ -78,8 +78,12 @@ type WSClient struct {
 // NewWSClient returns a new client. See the commentary on the func(*WSClient)
 // functions for a detailed description of how to configure ping period and
 // pong wait time. The endpoint argument must begin with a `/`.
+// The function panics if the provided address is invalid.
 func NewWSClient(remoteAddr, endpoint string, options ...func(*WSClient)) *WSClient {
-	protocol, addr, dialer := makeHTTPDialer(remoteAddr)
+	protocol, addr, err := toClientAddrAndParse(remoteAddr)
+	if err != nil {
+		panic(fmt.Sprintf("invalid remote %s: %s", remoteAddr, err))
+	}
 	// default to ws protocol, unless wss is explicitly specified
 	if protocol != "wss" {
 		protocol = "ws"
@@ -88,7 +92,7 @@ func NewWSClient(remoteAddr, endpoint string, options ...func(*WSClient)) *WSCli
 	c := &WSClient{
 		cdc:                  amino.NewCodec(),
 		Address:              addr,
-		Dialer:               dialer,
+		Dialer:               makeHTTPDialer(remoteAddr),
 		Endpoint:             endpoint,
 		PingPongLatencyTimer: metrics.NewTimer(),