Skip to content

Commit

Permalink
CBG-750: Backport JSON fixes for errors returned by API (#4524)
Browse files Browse the repository at this point in the history
* CBG-750 Backport CBG-661 to 2.7.2 - Fix error strings containing double quotes producing invalid JSON (#4437)

* Fix error strings containing double quotes producing invalid JSON

* Enhance 'no such database' test to verify output is JSON

* Fix incorrect test assertion for error message

* CBG-750 Backport CBG-661 to 2.7.2 - Fully escape string for JSON in writeStatus error handling (#4451)
  • Loading branch information
bbrks committed Mar 11, 2020
1 parent 6c2d01d commit 0920472
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 4 deletions.
6 changes: 6 additions & 0 deletions base/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ func ConvertJSONString(s string) string {
}
}

// ConvertToJSONString takes a string, and returns a JSON string, with any illegal characters escaped.
func ConvertToJSONString(s string) string {
b, _ := JSONMarshal(s)
return string(b)
}

// Concatenates and merges multiple string arrays into one, discarding all duplicates (including
// duplicates within a single array.) Ordering is preserved.
func MergeStringArrays(arrays ...[]string) (merged []string) {
Expand Down
50 changes: 48 additions & 2 deletions base/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,54 @@ func TestFixJSONNumbers(t *testing.T) {
}

func TestConvertJSONString(t *testing.T) {
goassert.Equals(t, ConvertJSONString(`"blah"`), "blah")
goassert.Equals(t, ConvertJSONString("blah"), "blah")
assert.Equal(t, "blah", ConvertJSONString(`"blah"`))
assert.Equal(t, "blah", ConvertJSONString("blah"))
}

func TestJSONStringUtils(t *testing.T) {
tests := []struct {
input string
output string
}{
{`test`, `"test"`},
{`"test"`, `"\"test\""`},
{"\x00", `"\u0000"`},
}

for _, test := range tests {
t.Run("ConvertToJSONString "+test.input, func(t *testing.T) {
out := ConvertToJSONString(test.input)
assert.Equal(t, test.output, out)
})
t.Run("ConvertJSONString "+test.input, func(t *testing.T) {
out := ConvertJSONString(test.output)
assert.Equal(t, test.input, out)
})
}
}

func BenchmarkJSONStringUtils(b *testing.B) {
tests := []struct {
input string
output string
}{
{`test`, `"test"`},
{`"test"`, `"\"test\""`},
{"\x00", `"\u0000"`},
}

for _, test := range tests {
b.Run("ConvertToJSONString "+test.input, func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = ConvertToJSONString(test.input)
}
})
b.Run("ConvertJSONString "+test.input, func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = ConvertJSONString(test.output)
}
})
}
}

func TestConvertBackQuotedStrings(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion rest/admin_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1897,7 +1897,9 @@ func TestHandleDeleteDB(t *testing.T) {
// Try to delete the database which doesn't exists
resp := rt.SendAdminRequest(http.MethodDelete, "/albums/", "{}")
assertStatus(t, resp, http.StatusNotFound)
assert.Contains(t, resp.Body.String(), "no such database")
assert.Contains(t, string(resp.BodyBytes()), "no such database")
var v map[string]interface{}
assert.NoError(t, json.Unmarshal(resp.BodyBytes(), &v), "couldn't unmarshal %s", string(resp.BodyBytes()))

// Create the database
resp = rt.SendAdminRequest(http.MethodPut, "/albums/", "{}")
Expand Down
3 changes: 2 additions & 1 deletion rest/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,8 @@ func (h *handler) writeStatus(status int, message string) {
h.setHeader("Content-Type", "application/json")
h.response.WriteHeader(status)
h.setStatus(status, message)
h.response.Write([]byte(`{"error":"` + errorStr + `","reason":"` + message + `"}`))

_, _ = h.response.Write([]byte(`{"error":"` + errorStr + `","reason":` + base.ConvertToJSONString(message) + `}`))
}

var kRangeRegex = regexp.MustCompile("^bytes=(\\d+)?-(\\d+)?$")
Expand Down

0 comments on commit 0920472

Please sign in to comment.