From 161e36fd56c2f95ad133dd03ddb33db0363ca742 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Fri, 28 Oct 2016 12:04:58 -0700 Subject: [PATCH 01/10] QuitService->BaseService --- client/ws_client.go | 8 ++++---- server/handlers.go | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/client/ws_client.go b/client/ws_client.go index 00e4222ad..4d975f8ec 100644 --- a/client/ws_client.go +++ b/client/ws_client.go @@ -19,7 +19,7 @@ const ( ) type WSClient struct { - QuitService + BaseService Address string // IP:PORT or /path/to/socket Endpoint string // /websocket/url/endpoint Dialer func(string, string) (net.Conn, error) @@ -39,7 +39,7 @@ func NewWSClient(remoteAddr, endpoint string) *WSClient { ResultsCh: make(chan json.RawMessage, wsResultsChannelCapacity), ErrorsCh: make(chan error, wsErrorsChannelCapacity), } - wsClient.QuitService = *NewQuitService(log, "WSClient", wsClient) + wsClient.BaseService = *NewBaseService(log, "WSClient", wsClient) return wsClient } @@ -48,7 +48,7 @@ func (wsc *WSClient) String() string { } func (wsc *WSClient) OnStart() error { - wsc.QuitService.OnStart() + wsc.BaseService.OnStart() err := wsc.dial() if err != nil { return err @@ -84,7 +84,7 @@ func (wsc *WSClient) dial() error { } func (wsc *WSClient) OnStop() { - wsc.QuitService.OnStop() + wsc.BaseService.OnStop() // ResultsCh/ErrorsCh is closed in receiveEventsRoutine. } diff --git a/server/handlers.go b/server/handlers.go index 2b2bb90ce..dbc5c6e75 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -264,7 +264,7 @@ const ( // contains listener id, underlying ws connection, // and the event switch for subscribing to events type wsConnection struct { - QuitService + BaseService remoteAddr string baseConn *websocket.Conn @@ -285,13 +285,13 @@ func NewWSConnection(baseConn *websocket.Conn, funcMap map[string]*RPCFunc, evsw funcMap: funcMap, evsw: evsw, } - wsc.QuitService = *NewQuitService(log, "wsConnection", wsc) + wsc.BaseService = *NewBaseService(log, "wsConnection", wsc) return wsc } // wsc.Start() blocks until the connection closes. func (wsc *wsConnection) OnStart() error { - wsc.QuitService.OnStart() + wsc.BaseService.OnStart() // Read subscriptions/unsubscriptions to events go wsc.readRoutine() @@ -318,7 +318,7 @@ func (wsc *wsConnection) OnStart() error { } func (wsc *wsConnection) OnStop() { - wsc.QuitService.OnStop() + wsc.BaseService.OnStop() wsc.evsw.RemoveListener(wsc.remoteAddr) wsc.readTimeout.Stop() wsc.pingTicker.Stop() From 34a806578ac0dd578c5595365a9c1478e0a689a0 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 2 Jan 2017 09:50:20 -0800 Subject: [PATCH 02/10] Handle hex strings and quoted strings in HTTP params Use 0x-prefixed hex strings in client server: Decode hex string args Encode all string args as 0x without trying to encode as JSON Added tests for special string arguments Fix server handling quoted string args Added string arg handling test cases to bash test script --- client/http_client.go | 19 ++++++++++++++++++- rpc_test.go | 36 ++++++++++++++++++++++++++++++++++++ server/handlers.go | 31 ++++++++++++++++++++++++++++++- test/test.sh | 31 +++++++++++++++++++++++++------ 4 files changed, 109 insertions(+), 8 deletions(-) mode change 100644 => 100755 test/test.sh diff --git a/client/http_client.go b/client/http_client.go index 816791b5c..54fbd1c2b 100644 --- a/client/http_client.go +++ b/client/http_client.go @@ -4,10 +4,12 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "io/ioutil" "net" "net/http" "net/url" + "reflect" "strings" . "github.com/tendermint/go-common" @@ -119,7 +121,7 @@ func (c *ClientURI) call(method string, params map[string]interface{}, result in if err != nil { return nil, err } - //log.Info(Fmt("URI request to %v (%v): %v", c.address, method, values)) + log.Info(Fmt("URI request to %v (%v): %v", c.address, method, values)) resp, err := c.client.PostForm(c.address+"/"+method, values) if err != nil { return nil, err @@ -176,6 +178,21 @@ func argsToJson(args map[string]interface{}) error { var n int var err error for k, v := range args { + // Convert strings to "0x"-prefixed hex + str, isString := reflect.ValueOf(v).Interface().(string) + if isString { + args[k] = fmt.Sprintf("0x%X", str) + continue + } + + // Convert byte slices to "0x"-prefixed hex + byteSlice, isByteSlice := reflect.ValueOf(v).Interface().([]byte) + if isByteSlice { + args[k] = fmt.Sprintf("0x%X", byteSlice) + continue + } + + // Pass everything else to go-wire buf := new(bytes.Buffer) wire.WriteJSON(v, buf, &n, &err) if err != nil { diff --git a/rpc_test.go b/rpc_test.go index 0c6e1dbdb..1fcda0e5e 100644 --- a/rpc_test.go +++ b/rpc_test.go @@ -164,3 +164,39 @@ func TestWS_UNIX(t *testing.T) { } testWS(t, cl) } + +func TestHexStringArg(t *testing.T) { + cl := rpcclient.NewClientURI(tcpAddr) + // should NOT be handled as hex + val := "0xabc" + params := map[string]interface{}{ + "arg": val, + } + var result Result + _, err := cl.Call("status", params, &result) + if err != nil { + t.Fatal(err) + } + got := result.(*ResultStatus).Value + if got != val { + t.Fatalf("Got: %v .... Expected: %v \n", got, val) + } +} + +func TestQuotedStringArg(t *testing.T) { + cl := rpcclient.NewClientURI(tcpAddr) + // should NOT be unquoted + val := "\"abc\"" + params := map[string]interface{}{ + "arg": val, + } + var result Result + _, err := cl.Call("status", params, &result) + if err != nil { + t.Fatal(err) + } + got := result.(*ResultStatus).Value + if got != val { + t.Fatalf("Got: %v .... Expected: %v \n", got, val) + } +} diff --git a/server/handlers.go b/server/handlers.go index dbc5c6e75..f5730213b 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -2,6 +2,7 @@ package rpcserver import ( "bytes" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -229,7 +230,35 @@ func httpParamsToArgs(rpcFunc *RPCFunc, r *http.Request) ([]reflect.Value, error for i, name := range argNames { ty := argTypes[i] arg := GetParam(r, name) - //log.Notice("param to arg", "ty", ty, "name", name, "arg", arg) + // log.Notice("param to arg", "ty", ty, "name", name, "arg", arg) + + // Handle quoted strings + if strings.HasPrefix(arg, "\"") && strings.HasSuffix(arg, "\"") { + data := arg[1 : len(arg)-1] + if ty.Kind() == reflect.String { + values[i] = reflect.ValueOf(string(data)) + } else { + values[i] = reflect.ValueOf(data) + } + continue + } + + // Handle hex strings + if strings.HasPrefix(strings.ToLower(arg), "0x") { + var value []byte + value, err = hex.DecodeString(arg[2:]) + if err != nil { + return nil, err + } + if ty.Kind() == reflect.String { + values[i] = reflect.ValueOf(string(value)) + } else { + values[i] = reflect.ValueOf(value) + } + continue + } + + // Pass values to go-wire values[i], err = _jsonStringToArg(ty, arg) if err != nil { return nil, err diff --git a/test/test.sh b/test/test.sh old mode 100644 new mode 100755 index 3d1f599dd..a5f8abd44 --- a/test/test.sh +++ b/test/test.sh @@ -6,18 +6,37 @@ go build -o server main.go PID=$! sleep 2 - +# simple JSONRPC request R1=`curl -s 'http://localhost:8008/hello_world?name="my_world"&num=5'` - - R2=`curl -s --data @data.json http://localhost:8008` +if [[ "$R1" != "$R2" ]]; then + echo "responses are not identical:" + echo "R1: $R1" + echo "R2: $R2" +else + echo "Success" +fi -kill -9 $PID +# request with 0x-prefixed hex string arg +R1=`curl -s 'http://localhost:8008/hello_world?name=0x41424344&num=123'` +R2='{"jsonrpc":"2.0","id":"","result":{"Result":"hi ABCD 123"},"error":""}' +if [[ "$R1" != "$R2" ]]; then + echo "responses are not identical:" + echo "R1: $R1" + echo "R2: $R2" +else + echo "Success" +fi +# request with unquoted string arg +R1=`curl -s 'http://localhost:8008/hello_world?name=abcd&num=123'` +R2="{\"jsonrpc\":\"2.0\",\"id\":\"\",\"result\":null,\"error\":\"Error converting http params to args: invalid character 'a' looking for beginning of value\"}" if [[ "$R1" != "$R2" ]]; then echo "responses are not identical:" echo "R1: $R1" echo "R2: $R2" - exit 1 +else + echo "Success" fi -echo "Success" + +kill -9 $PID From af1212897cfb1b2c43c02508e85babcf88fb5343 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Sat, 7 Jan 2017 14:00:27 -0800 Subject: [PATCH 03/10] Exit early in bash tests --- test/test.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test.sh b/test/test.sh index a5f8abd44..2db809948 100755 --- a/test/test.sh +++ b/test/test.sh @@ -13,6 +13,7 @@ if [[ "$R1" != "$R2" ]]; then echo "responses are not identical:" echo "R1: $R1" echo "R2: $R2" + exit 1 else echo "Success" fi @@ -24,6 +25,7 @@ if [[ "$R1" != "$R2" ]]; then echo "responses are not identical:" echo "R1: $R1" echo "R2: $R2" + exit 1 else echo "Success" fi @@ -35,6 +37,7 @@ if [[ "$R1" != "$R2" ]]; then echo "responses are not identical:" echo "R1: $R1" echo "R2: $R2" + exit 1 else echo "Success" fi From 86506cd4f83f67c307e5f32ad8ca39a337158fe8 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Sat, 7 Jan 2017 14:21:10 -0800 Subject: [PATCH 04/10] Handle quoted and hex string type HTTP args for both 'string' and '[]byte' type function args --- client/http_client.go | 9 +----- server/handlers.go | 66 ++++++++++++++++++++++++++++--------------- 2 files changed, 44 insertions(+), 31 deletions(-) diff --git a/client/http_client.go b/client/http_client.go index 54fbd1c2b..57da5d6ec 100644 --- a/client/http_client.go +++ b/client/http_client.go @@ -121,7 +121,7 @@ func (c *ClientURI) call(method string, params map[string]interface{}, result in if err != nil { return nil, err } - log.Info(Fmt("URI request to %v (%v): %v", c.address, method, values)) + // log.Info(Fmt("URI request to %v (%v): %v", c.address, method, values)) resp, err := c.client.PostForm(c.address+"/"+method, values) if err != nil { return nil, err @@ -178,13 +178,6 @@ func argsToJson(args map[string]interface{}) error { var n int var err error for k, v := range args { - // Convert strings to "0x"-prefixed hex - str, isString := reflect.ValueOf(v).Interface().(string) - if isString { - args[k] = fmt.Sprintf("0x%X", str) - continue - } - // Convert byte slices to "0x"-prefixed hex byteSlice, isByteSlice := reflect.ValueOf(v).Interface().([]byte) if isByteSlice { diff --git a/server/handlers.go b/server/handlers.go index f5730213b..7a4ec1a45 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -225,36 +225,18 @@ func httpParamsToArgs(rpcFunc *RPCFunc, r *http.Request) ([]reflect.Value, error argTypes := rpcFunc.args argNames := rpcFunc.argNames - var err error values := make([]reflect.Value, len(argNames)) for i, name := range argNames { ty := argTypes[i] arg := GetParam(r, name) // log.Notice("param to arg", "ty", ty, "name", name, "arg", arg) - // Handle quoted strings - if strings.HasPrefix(arg, "\"") && strings.HasSuffix(arg, "\"") { - data := arg[1 : len(arg)-1] - if ty.Kind() == reflect.String { - values[i] = reflect.ValueOf(string(data)) - } else { - values[i] = reflect.ValueOf(data) - } - continue + v, err, ok := nonJsonToArg(ty, arg) + if err != nil { + return nil, err } - - // Handle hex strings - if strings.HasPrefix(strings.ToLower(arg), "0x") { - var value []byte - value, err = hex.DecodeString(arg[2:]) - if err != nil { - return nil, err - } - if ty.Kind() == reflect.String { - values[i] = reflect.ValueOf(string(value)) - } else { - values[i] = reflect.ValueOf(value) - } + if ok { + values[i] = v continue } @@ -278,6 +260,44 @@ func _jsonStringToArg(ty reflect.Type, arg string) (reflect.Value, error) { return v, nil } +func nonJsonToArg(ty reflect.Type, arg string) (reflect.Value, error, bool) { + isQuotedString := strings.HasPrefix(arg, `"`) && strings.HasSuffix(arg, `"`) + isHexString := strings.HasPrefix(strings.ToLower(arg), "0x") + expectingString := ty.Kind() == reflect.String + expectingByteSlice := ty.Kind() == reflect.Slice && ty.Elem().Kind() == reflect.Uint8 + + if isHexString { + if !expectingString && !expectingByteSlice { + err := fmt.Errorf("Got a hex string arg, but expected '%s'", + ty.Kind().String()) + return reflect.ValueOf(nil), err, false + } + + var value []byte + value, err := hex.DecodeString(arg[2:]) + if err != nil { + return reflect.ValueOf(nil), err, false + } + if ty.Kind() == reflect.String { + return reflect.ValueOf(string(value)), nil, true + } + return reflect.ValueOf([]byte(value)), nil, true + } + + if isQuotedString && expectingByteSlice { + var err error + v := reflect.New(reflect.TypeOf("")) + wire.ReadJSONPtr(v.Interface(), []byte(arg), &err) + if err != nil { + return reflect.ValueOf(nil), err, false + } + v = v.Elem() + return reflect.ValueOf([]byte(v.String())), nil, true + } + + return reflect.ValueOf(nil), nil, false +} + // rpc.http //----------------------------------------------------------------------------- // rpc.websocket From 4d7aa62a10068450f977ebe786ea1412bf633658 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Sat, 7 Jan 2017 14:21:49 -0800 Subject: [PATCH 05/10] Added test for unexpected hex string type HTTP args --- test/test.sh | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/test.sh b/test/test.sh index 2db809948..dc22ad2f3 100755 --- a/test/test.sh +++ b/test/test.sh @@ -6,7 +6,7 @@ go build -o server main.go PID=$! sleep 2 -# simple JSONRPC request +# simple request R1=`curl -s 'http://localhost:8008/hello_world?name="my_world"&num=5'` R2=`curl -s --data @data.json http://localhost:8008` if [[ "$R1" != "$R2" ]]; then @@ -42,4 +42,16 @@ else echo "Success" fi -kill -9 $PID +# request with string type when expecting number arg +R1=`curl -s 'http://localhost:8008/hello_world?name="abcd"&num=0xabcd'` +R2="{\"jsonrpc\":\"2.0\",\"id\":\"\",\"result\":null,\"error\":\"Error converting http params to args: Got a 'hex string' arg, but expected 'int'\"}" +if [[ "$R1" != "$R2" ]]; then + echo "responses are not identical:" + echo "R1: $R1" + echo "R2: $R2" + exit 1 +else + echo "Success" +fi + +kill -9 $PID || exit 0 From 0eb278ad3b6ca0a1fcb7f012c02360bfb77da8ee Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 12 Jan 2017 00:13:20 -0500 Subject: [PATCH 06/10] version bump 0.6.0 --- version.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.go b/version.go index edf8d40da..33eb7fe51 100644 --- a/version.go +++ b/version.go @@ -1,7 +1,7 @@ package rpc const Maj = "0" -const Min = "5" // refactored out of tendermint/tendermint; RPCResponse.Result is RawJSON -const Fix = "1" // support tcp:// or unix:// prefixes +const Min = "6" // 0x-prefixed string args handled as hex +const Fix = "0" // const Version = Maj + "." + Min + "." + Fix From 94fed25975c31e5d405369f0e3558da3cff85c2b Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 12 Jan 2017 10:22:23 -0500 Subject: [PATCH 07/10] fix test --- test/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.sh b/test/test.sh index dc22ad2f3..c38059016 100755 --- a/test/test.sh +++ b/test/test.sh @@ -44,7 +44,7 @@ fi # request with string type when expecting number arg R1=`curl -s 'http://localhost:8008/hello_world?name="abcd"&num=0xabcd'` -R2="{\"jsonrpc\":\"2.0\",\"id\":\"\",\"result\":null,\"error\":\"Error converting http params to args: Got a 'hex string' arg, but expected 'int'\"}" +R2="{\"jsonrpc\":\"2.0\",\"id\":\"\",\"result\":null,\"error\":\"Error converting http params to args: Got a hex string arg, but expected 'int'\"}" if [[ "$R1" != "$R2" ]]; then echo "responses are not identical:" echo "R1: $R1" From 08f2b5bc84f6c7b8151bd19a1d90da98207ea805 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 12 Jan 2017 21:46:50 -0500 Subject: [PATCH 08/10] get deps for testing --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 4cc7c1594..e73849878 100644 --- a/Makefile +++ b/Makefile @@ -2,10 +2,10 @@ all: test -test: +test: get_deps go test --race github.com/tendermint/go-rpc/... cd ./test && bash test.sh get_deps: - go get -t -d github.com/tendermint/go-rpc/... + go get -t -u github.com/tendermint/go-rpc/... From ac443fa61f30bbc65d7abfe71e897caaa9338bbc Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 12 Jan 2017 21:59:02 -0500 Subject: [PATCH 09/10] run tests from bash script --- Makefile | 6 ++---- circle.yml | 4 ---- test/test.sh | 12 ++++++++++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index e73849878..3b005ea36 100644 --- a/Makefile +++ b/Makefile @@ -2,10 +2,8 @@ all: test -test: get_deps - go test --race github.com/tendermint/go-rpc/... - cd ./test && bash test.sh - +test: + bash ./test/test.sh get_deps: go get -t -u github.com/tendermint/go-rpc/... diff --git a/circle.yml b/circle.yml index f022d5a6b..1b3c8c609 100644 --- a/circle.yml +++ b/circle.yml @@ -14,10 +14,6 @@ checkout: # - git submodule sync # - git submodule update --init # use submodules -dependencies: - override: - - "cd $REPO && make get_deps" - test: override: - "cd $REPO && make test" diff --git a/test/test.sh b/test/test.sh index c38059016..f5e740241 100755 --- a/test/test.sh +++ b/test/test.sh @@ -1,4 +1,16 @@ #! /bin/bash + +cd $GOPATH/src/github.com/tendermint/go-rpc + +# get deps +go get -u -t ./... + +# go tests +go test --race github.com/tendermint/go-rpc/... + + +# integration tests +cd test set -e go build -o server main.go From 6177eb8398ebd4613fbecb71fd96d7c7d97303ec Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 12 Jan 2017 22:01:20 -0500 Subject: [PATCH 10/10] love you circley --- circle.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/circle.yml b/circle.yml index 1b3c8c609..99af678c6 100644 --- a/circle.yml +++ b/circle.yml @@ -14,6 +14,10 @@ checkout: # - git submodule sync # - git submodule update --init # use submodules +dependencies: + override: + - "cd $REPO" + test: override: - "cd $REPO && make test"