From aa721b972fdd764d497dc284e6d580a0863408fd Mon Sep 17 00:00:00 2001 From: gracenoah Date: Thu, 5 Sep 2019 16:13:22 +0300 Subject: [PATCH] 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 --- CHANGELOG_PENDING.md | 2 + rpc/client/httpclient.go | 14 ++++- rpc/client/rpc_test.go | 18 ++++++ rpc/lib/client/http_client.go | 104 ++++++++++++++++++++++++++-------- rpc/lib/client/ws_client.go | 8 ++- 5 files changed, 118 insertions(+), 28 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 7c8ca225..1b3cbc06 100644 --- a/CHANGELOG_PENDING.md +++ b/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: diff --git a/rpc/client/httpclient.go b/rpc/client/httpclient.go index c4334523..d1981e1c 100644 --- a/rpc/client/httpclient.go +++ b/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 ://: and // the websocket path (which always seems to be "/websocket") +// The function panics if the provided remote is invalid. 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) diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index de5e18f1..4bb37cf4 100644 --- a/rpc/client/rpc_test.go +++ b/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) diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index db57c536..28f51191 100644 --- a/rpc/lib/client/http_client.go +++ b/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(), } } diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index 05180c75..1779e9db 100644 --- a/rpc/lib/client/ws_client.go +++ b/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(),