From 0920472b9006c9bb4c22175a95dafca5eb097fa3 Mon Sep 17 00:00:00 2001
From: Ben Brooks <ben.brooks@couchbase.com>
Date: Wed, 11 Mar 2020 00:12:50 +0000
Subject: [PATCH] CBG-750: Backport JSON fixes for errors returned by API
 (#4524)

* 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)
---
 base/util.go           |  6 +++++
 base/util_test.go      | 50 ++++++++++++++++++++++++++++++++++++++++--
 rest/admin_api_test.go |  4 +++-
 rest/handler.go        |  3 ++-
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/base/util.go b/base/util.go
index 4d37510be8..86a72e82ab 100644
--- a/base/util.go
+++ b/base/util.go
@@ -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) {
diff --git a/base/util_test.go b/base/util_test.go
index 23274bc76b..5b33db7fdd 100644
--- a/base/util_test.go
+++ b/base/util_test.go
@@ -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) {
diff --git a/rest/admin_api_test.go b/rest/admin_api_test.go
index 4b5ff89ce7..ef669834c3 100644
--- a/rest/admin_api_test.go
+++ b/rest/admin_api_test.go
@@ -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/", "{}")
diff --git a/rest/handler.go b/rest/handler.go
index 24ffb1c6d5..fb30c59989 100644
--- a/rest/handler.go
+++ b/rest/handler.go
@@ -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+)?$")