rpc: fix byte string decoding for URL parameters (#8431)

In #8397 I tried to remove all the cases where we needed to keep track of the
target type of parameters for JSON encoding, but there is one case still left:
When decoding parameters from URL query terms, there is no way to tell whether
or not we need base64 encoding without knowing whether the underlying type of
the target is string or []byte.

To fix this, keep track of parameters that are []byte valued when RPCFunc is
compiling its argument map, and use that when parsing URL query terms.  Update
the tests accordingly.

rpc/jsonrpc/server/http_json_handler.go

@@ -118,17 +118,15 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st
 	noArgs := make(map[string]string)
 	for name, rf := range funcMap {
 		base := fmt.Sprintf("//%s/%s", r.Host, name)
-		// N.B. Check argNames, not args, since the type list includes the type
-		// of the leading context argument.
-		if len(rf.argNames) == 0 {
+		if len(rf.args) == 0 {
 			noArgs[name] = base
-		} else {
-			query := append([]string(nil), rf.argNames...)
-			for i, arg := range query {
-				query[i] = arg + "=_"
-			}
-			hasArgs[name] = base + "?" + strings.Join(query, "&")
+			continue
 		}
+		var query []string
+		for _, arg := range rf.args {
+			query = append(query, arg.name+"=_")
+		}
+		hasArgs[name] = base + "?" + strings.Join(query, "&")
 	}
 	w.Header().Set("Content-Type", "text/html")
 	_ = listOfEndpoints.Execute(w, map[string]map[string]string{

rpc/jsonrpc/server/http_uri_handler.go

@@ -23,7 +23,7 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit
 		ctx := rpctypes.WithCallInfo(req.Context(), &rpctypes.CallInfo{
 			HTTPRequest: req,
 		})
-		args, err := parseURLParams(rpcFunc.argNames, req)
+		args, err := parseURLParams(rpcFunc.args, req)
 		if err != nil {
 			w.Header().Set("Content-Type", "text/plain")
 			w.WriteHeader(http.StatusBadRequest)
@@ -40,7 +40,7 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit
 	}
 }
 
-func parseURLParams(argNames []string, req *http.Request) ([]byte, error) {
+func parseURLParams(args []argInfo, req *http.Request) ([]byte, error) {
 	if err := req.ParseForm(); err != nil {
 		return nil, fmt.Errorf("invalid HTTP request: %w", err)
 	}
@@ -52,15 +52,15 @@ func parseURLParams(argNames []string, req *http.Request) ([]byte, error) {
 	}
 
 	params := make(map[string]interface{})
-	for _, name := range argNames {
-		v, ok := getArg(name)
+	for _, arg := range args {
+		v, ok := getArg(arg.name)
 		if !ok {
 			continue
 		}
 		if z, err := decodeInteger(v); err == nil {
-			params[name] = z
+			params[arg.name] = z
 		} else if b, err := strconv.ParseBool(v); err == nil {
-			params[name] = b
+			params[arg.name] = b
 		} else if lc := strings.ToLower(v); strings.HasPrefix(lc, "0x") {
 			dec, err := hex.DecodeString(lc[2:])
 			if err != nil {
@@ -68,15 +68,23 @@ func parseURLParams(argNames []string, req *http.Request) ([]byte, error) {
 			} else if len(dec) == 0 {
 				return nil, errors.New("invalid empty hex string")
 			}
-			params[name] = dec
+			if arg.isBinary {
+				params[arg.name] = dec
+			} else {
+				params[arg.name] = string(dec)
+			}
 		} else if isQuotedString(v) {
 			var dec string
 			if err := json.Unmarshal([]byte(v), &dec); err != nil {
 				return nil, fmt.Errorf("invalid quoted string: %w", err)
 			}
-			params[name] = dec
+			if arg.isBinary {
+				params[arg.name] = []byte(dec)
+			} else {
+				params[arg.name] = dec
+			}
 		} else {
-			params[name] = v
+			params[arg.name] = v
 		}
 	}
 	return json.Marshal(params)

rpc/jsonrpc/server/parse_test.go

@@ -188,44 +188,50 @@ func TestParseURI(t *testing.T) {
 		tests := []struct {
 			name string
 			url  string
-			args []string
+			args []argInfo
 			want string
 			fail bool
 		}{
 			{
 				name: "quoted numbers and strings",
 				url:  `http://localhost?num="7"&str="flew"&neg="-10"`,
-				args: []string{"neg", "num", "str", "other"},
+				args: []argInfo{{name: "neg"}, {name: "num"}, {name: "str"}, {name: "other"}},
 				want: `{"neg":-10,"num":7,"str":"flew"}`,
 			},
 			{
 				name: "unquoted numbers and strings",
 				url:  `http://localhost?num1=7&str1=cabbage&num2=-199&str2=hey+you`,
-				args: []string{"num1", "num2", "str1", "str2", "other"},
+				args: []argInfo{{name: "num1"}, {name: "num2"}, {name: "str1"}, {name: "str2"}, {name: "other"}},
 				want: `{"num1":7,"num2":-199,"str1":"cabbage","str2":"hey you"}`,
 			},
 			{
-				name: "byte strings in hex",
+				name: "quoted byte strings",
+				url:  `http://localhost?left="Fahrvergnügen"&right="Applesauce"`,
+				args: []argInfo{{name: "left", isBinary: true}, {name: "right", isBinary: false}},
+				want: `{"left":"RmFocnZlcmduw7xnZW4=","right":"Applesauce"}`,
+			},
+			{
+				name: "hexadecimal byte strings",
 				url:  `http://localhost?lower=0x626f62&upper=0X646F7567`,
-				args: []string{"upper", "lower", "other"},
-				want: `{"lower":"Ym9i","upper":"ZG91Zw=="}`,
+				args: []argInfo{{name: "upper", isBinary: true}, {name: "lower", isBinary: false}, {name: "other"}},
+				want: `{"lower":"bob","upper":"ZG91Zw=="}`,
 			},
 			{
 				name: "invalid hex odd length",
 				url:  `http://localhost?bad=0xa`,
-				args: []string{"bad", "superbad"},
+				args: []argInfo{{name: "bad"}, {name: "superbad"}},
 				fail: true,
 			},
 			{
 				name: "invalid hex empty",
 				url:  `http://localhost?bad=0x`,
-				args: []string{"bad"},
+				args: []argInfo{{name: "bad"}},
 				fail: true,
 			},
 			{
 				name: "invalid quoted string",
 				url:  `http://localhost?bad="double""`,
-				args: []string{"bad"},
+				args: []argInfo{{name: "bad"}},
 				fail: true,
 			},
 		}
@@ -305,7 +311,7 @@ func TestParseURI(t *testing.T) {
 				if err != nil {
 					t.Fatalf("NewRequest for %q: %v", test.url, err)
 				}
-				bits, err := parseURLParams(echo.argNames, hreq)
+				bits, err := parseURLParams(echo.args, hreq)
 				if err != nil {
 					t.Fatalf("Parse %#q: unexpected error: %v", test.url, err)
 				}

rpc/jsonrpc/server/rpc_func.go

@@ -32,11 +32,20 @@ func RegisterRPCFuncs(mux *http.ServeMux, funcMap map[string]*RPCFunc, logger lo
 
 // RPCFunc contains the introspected type information for a function.
 type RPCFunc struct {
-	f        reflect.Value // underlying rpc function
-	param    reflect.Type  // the parameter struct, or nil
-	result   reflect.Type  // the non-error result type, or nil
-	argNames []string      // name of each argument (for display)
-	ws       bool          // websocket only
+	f      reflect.Value // underlying rpc function
+	param  reflect.Type  // the parameter struct, or nil
+	result reflect.Type  // the non-error result type, or nil
+	args   []argInfo     // names and type information (for URL decoding)
+	ws     bool          // websocket only
+}
+
+// argInfo records the name of a field, along with a bit to tell whether the
+// value of the field requires binary data, having underlying type []byte.  The
+// flag is needed when decoding URL parameters, where we permit quoted strings
+// to be passed for either argument type.
+type argInfo struct {
+	name     string
+	isBinary bool // value wants binary data
 }
 
 // Call parses the given JSON parameters and calls the function wrapped by rf
@@ -96,12 +105,12 @@ func (rf *RPCFunc) adjustParams(data []byte) (json.RawMessage, error) {
 		var args []json.RawMessage
 		if err := json.Unmarshal(base, &args); err != nil {
 			return nil, err
-		} else if len(args) != len(rf.argNames) {
-			return nil, fmt.Errorf("got %d arguments, want %d", len(args), len(rf.argNames))
+		} else if len(args) != len(rf.args) {
+			return nil, fmt.Errorf("got %d arguments, want %d", len(args), len(rf.args))
 		}
 		m := make(map[string]json.RawMessage)
 		for i, arg := range args {
-			m[rf.argNames[i]] = arg
+			m[rf.args[i].name] = arg
 		}
 		return json.Marshal(m)
 	} else if bytes.HasPrefix(base, []byte("{")) || bytes.Equal(base, []byte("null")) {
@@ -180,12 +189,15 @@ func newRPCFunc(f interface{}) (*RPCFunc, error) {
 		rtype = ft.Out(0)
 	}
 
-	var argNames []string
+	var args []argInfo
 	if ptype != nil {
 		for i := 0; i < ptype.NumField(); i++ {
 			field := ptype.Field(i)
 			if tag := strings.SplitN(field.Tag.Get("json"), ",", 2)[0]; tag != "" && tag != "-" {
-				argNames = append(argNames, tag)
+				args = append(args, argInfo{
+					name:     tag,
+					isBinary: isByteArray(field.Type),
+				})
 			} else if tag == "-" {
 				// If the tag is "-" the field should explicitly be ignored, even
 				// if it is otherwise eligible.
@@ -194,16 +206,19 @@ func newRPCFunc(f interface{}) (*RPCFunc, error) {
 				// Note that this is an aesthetic choice; the standard decoder will
 				// match without regard to case anyway.
 				name := strings.ToLower(field.Name[:1]) + field.Name[1:]
-				argNames = append(argNames, name)
+				args = append(args, argInfo{
+					name:     name,
+					isBinary: isByteArray(field.Type),
+				})
 			}
 		}
 	}
 
 	return &RPCFunc{
-		f:        fv,
-		param:    ptype,
-		result:   rtype,
-		argNames: argNames,
+		f:      fv,
+		param:  ptype,
+		result: rtype,
+		args:   args,
 	}, nil
 }
 
@@ -225,3 +240,8 @@ func isNullOrEmpty(params json.RawMessage) bool {
 		bytes.Equal(params, []byte("{}")) ||
 		bytes.Equal(params, []byte("[]"))
 }
+
+// isByteArray reports whether t is (equivalent to) []byte.
+func isByteArray(t reflect.Type) bool {
+	return t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Uint8
+}