From 34a806578ac0dd578c5595365a9c1478e0a689a0 Mon Sep 17 00:00:00 2001 From: Matt Bell Date: Mon, 2 Jan 2017 09:50:20 -0800 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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