From afeb8e6f6a89ea9b6074cb5382e1825a938baeeb Mon Sep 17 00:00:00 2001 From: Tolga Ceylan Date: Fri, 9 Mar 2018 11:59:30 -0800 Subject: [PATCH] fn: json excess data check should ignore whitespace (#830) * fn: json excess data check should ignore whitespace * fn: adjustments and test case --- api/agent/protocol/json.go | 16 +++++++++++++--- api/server/runner_test.go | 18 ++++++++++++------ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/api/agent/protocol/json.go b/api/agent/protocol/json.go index eb0b911b9..8b9490085 100644 --- a/api/agent/protocol/json.go +++ b/api/agent/protocol/json.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "sync" + "unicode" "go.opencensus.io/trace" @@ -149,9 +150,18 @@ func (h *JSONProtocol) Dispatch(ctx context.Context, ci CallInfo, w io.Writer) e func (h *JSONProtocol) isExcessData(err error, decoder *json.Decoder) error { if err == nil { // Now check for excess output, if this is the case, we can be certain that the next request will fail. - tmp, ok := decoder.Buffered().(*bytes.Reader) - if ok && tmp.Len() > 0 { - return ErrExcessData + reader, ok := decoder.Buffered().(*bytes.Reader) + if ok && reader.Len() > 0 { + // Let's check if extra data is whitespace, which is valid/ignored in json + for { + r, _, err := reader.ReadRune() + if err == io.EOF { + break + } + if !unicode.IsSpace(r) { + return ErrExcessData + } + } } } return err diff --git a/api/server/runner_test.go b/api/server/runner_test.go index ed2c43ef5..19138e8b5 100644 --- a/api/server/runner_test.go +++ b/api/server/runner_test.go @@ -186,6 +186,7 @@ func TestRouteRunnerIOPipes(t *testing.T) { // sleep between logs and with debug enabled, fn-test-utils will log header/footer below: immediateGarbage := `{"isDebug": true, "postOutGarbage": "YOGURT_YOGURT_YOGURT", "postSleepTime": 0}` + immediateJsonValidGarbage := `{"isDebug": true, "postOutGarbage": "\r", "postSleepTime": 0}` delayedGarbage := `{"isDebug": true, "postOutGarbage": "YOGURT_YOGURT_YOGURT", "postSleepTime": 1000}` ok := `{"isDebug": true}` @@ -203,6 +204,9 @@ func TestRouteRunnerIOPipes(t *testing.T) { // // JSON WORLD // + // this should go through. + {"/r/zoo/json/", immediateJsonValidGarbage, "GET", http.StatusOK, "", nil, 0}, + // CASE I: immediate garbage: likely to be in the json decoder buffer after json resp parsing {"/r/zoo/json/", immediateGarbage, "GET", http.StatusOK, "", nil, 0}, @@ -275,20 +279,22 @@ func TestRouteRunnerIOPipes(t *testing.T) { t.Logf("Test %d: dockerId: %v", i, containerIds[i]) time.Sleep(test.sleepAmount) } - jsonIds := containerIds[0:4] + + jsonIds := containerIds[0:5] // now cross check JSON container ids: - if jsonIds[0] != jsonIds[1] && - jsonIds[2] == "N/A" && - jsonIds[1] != jsonIds[2] && - jsonIds[2] != jsonIds[3] { + if jsonIds[1] != jsonIds[2] && + jsonIds[0] == jsonIds[1] && + jsonIds[3] == "N/A" && + jsonIds[2] != jsonIds[3] && + jsonIds[3] != jsonIds[4] { t.Logf("json container ids are OK, ids=%v", jsonIds) } else { isFailure = true t.Errorf("json container ids are not OK, ids=%v", jsonIds) } - httpids := containerIds[4:] + httpids := containerIds[5:] // now cross check HTTP container ids: if httpids[0] == httpids[1] &&