From: Tom Clegg <tclegg@veritasgenetics.com>
Date: Tue, 26 Nov 2019 21:16:27 +0000 (-0500)
Subject: Merge branch '15877-accept-json-in-json'
X-Git-Tag: 2.0.0~99
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/5338c3fe0abbc6599aa290085be13eecfb0044e9?hp=c884807d0bf8c32bb220ba382c97a455ade4bd0a

Merge branch '15877-accept-json-in-json'

fixes #15877

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>
---

diff --git a/lib/controller/router/request.go b/lib/controller/router/request.go
index 4d18395b6a..cc63794868 100644
--- a/lib/controller/router/request.go
+++ b/lib/controller/router/request.go
@@ -16,6 +16,45 @@ import (
 	"github.com/gorilla/mux"
 )
 
+func guessAndParse(k, v string) (interface{}, error) {
+	// All of these form values arrive as strings, so we need some
+	// type-guessing to accept non-string inputs:
+	//
+	// Values for parameters that take ints (limit=1) or bools
+	// (include_trash=1) are parsed accordingly.
+	//
+	// "null" and "" are nil.
+	//
+	// Values that look like JSON objects, arrays, or strings are
+	// parsed as JSON.
+	//
+	// The rest are left as strings.
+	switch {
+	case intParams[k]:
+		return strconv.ParseInt(v, 10, 64)
+	case boolParams[k]:
+		return stringToBool(v), nil
+	case v == "null" || v == "":
+		return nil, nil
+	case strings.HasPrefix(v, "["):
+		var j []interface{}
+		err := json.Unmarshal([]byte(v), &j)
+		return j, err
+	case strings.HasPrefix(v, "{"):
+		var j map[string]interface{}
+		err := json.Unmarshal([]byte(v), &j)
+		return j, err
+	case strings.HasPrefix(v, "\""):
+		var j string
+		err := json.Unmarshal([]byte(v), &j)
+		return j, err
+	default:
+		return v, nil
+	}
+	// TODO: Need to accept "?foo[]=bar&foo[]=baz" as
+	// foo=["bar","baz"]?
+}
+
 // Parse req as an Arvados V1 API request and return the request
 // parameters.
 //
@@ -33,56 +72,11 @@ func (rtr *router) loadRequestParams(req *http.Request, attrsKey string) (map[st
 	// Content-Type is application/x-www-form-urlencoded -- the
 	// request body.
 	for k, values := range req.Form {
-		// All of these form values arrive as strings, so we
-		// need some type-guessing to accept non-string
-		// inputs:
-		//
-		// Values for parameters that take ints (limit=1) or
-		// bools (include_trash=1) are parsed accordingly.
-		//
-		// "null" and "" are nil.
-		//
-		// Values that look like JSON objects, arrays, or
-		// strings are parsed as JSON.
-		//
-		// The rest are left as strings.
 		for _, v := range values {
-			switch {
-			case intParams[k]:
-				params[k], err = strconv.ParseInt(v, 10, 64)
-				if err != nil {
-					return nil, err
-				}
-			case boolParams[k]:
-				params[k] = stringToBool(v)
-			case v == "null" || v == "":
-				params[k] = nil
-			case strings.HasPrefix(v, "["):
-				var j []interface{}
-				err := json.Unmarshal([]byte(v), &j)
-				if err != nil {
-					return nil, err
-				}
-				params[k] = j
-			case strings.HasPrefix(v, "{"):
-				var j map[string]interface{}
-				err := json.Unmarshal([]byte(v), &j)
-				if err != nil {
-					return nil, err
-				}
-				params[k] = j
-			case strings.HasPrefix(v, "\""):
-				var j string
-				err := json.Unmarshal([]byte(v), &j)
-				if err != nil {
-					return nil, err
-				}
-				params[k] = j
-			default:
-				params[k] = v
+			params[k], err = guessAndParse(k, v)
+			if err != nil {
+				return nil, err
 			}
-			// TODO: Need to accept "?foo[]=bar&foo[]=baz"
-			// as foo=["bar","baz"]?
 		}
 	}
 
@@ -98,7 +92,20 @@ func (rtr *router) loadRequestParams(req *http.Request, attrsKey string) (map[st
 			return nil, httpError(http.StatusBadRequest, err)
 		}
 		for k, v := range jsonParams {
-			params[k] = v
+			switch v := v.(type) {
+			case string:
+				// The Ruby "arv" cli tool sends a
+				// JSON-encode params map with
+				// JSON-encoded values.
+				dec, err := guessAndParse(k, v)
+				if err != nil {
+					return nil, err
+				}
+				jsonParams[k] = dec
+				params[k] = dec
+			default:
+				params[k] = v
+			}
 		}
 		if attrsKey != "" && params[attrsKey] == nil {
 			// Copy top-level parameters from JSON request
diff --git a/lib/controller/router/request_test.go b/lib/controller/router/request_test.go
index 89238f6563..118415cb40 100644
--- a/lib/controller/router/request_test.go
+++ b/lib/controller/router/request_test.go
@@ -49,10 +49,26 @@ func (tr *testReq) Request() *http.Request {
 	} else if tr.json {
 		if tr.jsonAttrsTop {
 			for k, v := range tr.attrs {
-				param[k] = v
+				if tr.jsonStringParam {
+					j, err := json.Marshal(v)
+					if err != nil {
+						panic(err)
+					}
+					param[k] = string(j)
+				} else {
+					param[k] = v
+				}
 			}
 		} else if tr.attrs != nil {
-			param[tr.attrsKey] = tr.attrs
+			if tr.jsonStringParam {
+				j, err := json.Marshal(tr.attrs)
+				if err != nil {
+					panic(err)
+				}
+				param[tr.attrsKey] = string(j)
+			} else {
+				param[tr.attrsKey] = tr.attrs
+			}
 		}
 		tr.body = bytes.NewBuffer(nil)
 		err := json.NewEncoder(tr.body).Encode(param)
@@ -118,6 +134,8 @@ func (s *RouterSuite) TestAttrsInBody(c *check.C) {
 	for _, tr := range []testReq{
 		{attrsKey: "model_name", json: true, attrs: attrs},
 		{attrsKey: "model_name", json: true, attrs: attrs, jsonAttrsTop: true},
+		{attrsKey: "model_name", json: true, attrs: attrs, jsonAttrsTop: true, jsonStringParam: true},
+		{attrsKey: "model_name", json: true, attrs: attrs, jsonAttrsTop: false, jsonStringParam: true},
 	} {
 		c.Logf("tr: %#v", tr)
 		req := tr.Request()