json: don't copy strings while escaping

lib/json/json.c

@@ -640,49 +640,6 @@ static int json_escape_internal(const char *str,
 	return ret;
 }
 
-struct appender {
-	char *buffer;
-	size_t used;
-	size_t size;
-};
-
-static int append_bytes_to_buf(const u8_t *bytes, size_t len, void *data)
-{
-	struct appender *appender = data;
-
-	if (len > appender->size - appender->used) {
-		return -ENOMEM;
-	}
-
-	memcpy(appender->buffer + appender->used, bytes, len);
-	appender->used += len;
-	appender->buffer[appender->used] = '\0';
-
-	return 0;
-}
-
-static int json_escape_buf(char *str, size_t *len, size_t buf_size)
-{
-	char tmp_buf[buf_size + 1];
-	struct appender appender = { .buffer = tmp_buf, .size = buf_size };
-	int ret;
-
-	ret = json_escape_internal(str, append_bytes_to_buf, &appender);
-	if (ret < 0) {
-		return ret;
-	}
-
-	ret = append_bytes_to_buf("", 1, &appender);
-	if (!ret) {
-		memcpy(str, tmp_buf, appender.size);
-		if (len) {
-			*len = appender.size;
-		}
-	}
-
-	return ret;
-}
-
 size_t json_calc_escaped_len(const char *str, size_t len)
 {
 	size_t escaped_len = len;
@@ -699,13 +656,13 @@ size_t json_calc_escaped_len(const char *str, size_t len)
 
 ssize_t json_escape(char *str, size_t *len, size_t buf_size)
 {
-	size_t escaped_len;
-
-	escaped_len = json_calc_escaped_len(str, *len);
+	char *next; /* Points after next character to escape. */
+	char *dest; /* Points after next place to write escaped character. */
+	size_t escaped_len = json_calc_escaped_len(str, *len);
 
 	if (escaped_len == *len) {
-		/* If no escape is necessary, don't bother using up temporary
-		 * stack space to copy the string.
+		/*
+		 * If no escape is necessary, there is nothing to do.
 		 */
 		return 0;
 	}
@@ -714,7 +671,27 @@ ssize_t json_escape(char *str, size_t *len, size_t buf_size)
 		return -ENOMEM;
 	}
 
-	return json_escape_buf(str, len, escaped_len);
+	/*
+	 * By walking backwards in the buffer from the end positions
+	 * of both the original and escaped strings, we avoid using
+	 * extra space. Characters in the original string are
+	 * overwritten only after they have already been escaped.
+	 */
+	str[escaped_len] = '\0';
+	for (next = &str[*len], dest = &str[escaped_len]; next != str;) {
+		char next_c = *(--next);
+		char escape = escape_as(next_c);
+
+		if (escape) {
+			*(--dest) = escape;
+			*(--dest) = '\\';
+		} else {
+			*(--dest) = next_c;
+		}
+	}
+	*len = escaped_len;
+
+	return 0;
 }
 
 static int encode(const struct json_obj_descr *descr, const void *val,
@@ -881,6 +858,27 @@ int json_obj_encode(const struct json_obj_descr *descr, size_t descr_len,
 	return append_bytes("", 1, data);
 }
 
+struct appender {
+	char *buffer;
+	size_t used;
+	size_t size;
+};
+
+static int append_bytes_to_buf(const u8_t *bytes, size_t len, void *data)
+{
+	struct appender *appender = data;
+
+	if (len > appender->size - appender->used) {
+		return -ENOMEM;
+	}
+
+	memcpy(appender->buffer + appender->used, bytes, len);
+	appender->used += len;
+	appender->buffer[appender->used] = '\0';
+
+	return 0;
+}
+
 int json_obj_encode_buf(const struct json_obj_descr *descr, size_t descr_len,
 			const void *val, char *buffer, size_t buf_size)
 {

tests/lib/json/src/main.c

@@ -171,6 +171,91 @@ static void test_json_key_not_in_descr(void)
 	zassert_equal(ret, 0, "No items should be decoded");
 }
 
+static void test_json_escape(void)
+{
+	char buf[42];
+	char string[] = "\"abc"
+			"\\1`23"
+			"\bf'oo"
+			"\fbar"
+			"\nbaz"
+			"\rquux"
+			"\tfred\"";
+	const char *expected = "\\\"abc"
+			       "\\\\1`23"
+			       "\\bf'oo"
+			       "\\fbar"
+			       "\\nbaz"
+			       "\\rquux"
+			       "\\tfred\\\"";
+	size_t len;
+	ssize_t ret;
+
+	strncpy(buf, string, sizeof(buf) - 1);
+	len = strlen(buf);
+
+	ret = json_escape(buf, &len, sizeof(buf));
+	zassert_equal(ret, 0, "Escape succeeded");
+	zassert_equal(len, sizeof(buf) - 1,
+		      "Escaped length computed correctly");
+	zassert_true(!strcmp(buf, expected),
+		     "Escaped value is correct");
+}
+
+/* Edge case: only one character, which must be escaped. */
+static void test_json_escape_one(void)
+{
+	char buf[3] = {'\t', '\0', '\0'};
+	const char *expected = "\\t";
+	size_t len = strlen(buf);
+	ssize_t ret;
+
+	ret = json_escape(buf, &len, sizeof(buf));
+	zassert_equal(ret, 0,
+		      "Escaping one character succeded");
+	zassert_equal(len, sizeof(buf) - 1,
+		      "Escaping one character length is correct");
+	zassert_true(!strcmp(buf, expected),
+		     "Escaped value is correct");
+}
+
+static void test_json_escape_empty(void)
+{
+	char empty[] = "";
+	size_t len = sizeof(empty) - 1;
+	ssize_t ret;
+
+	ret = json_escape(empty, &len, sizeof(empty));
+	zassert_equal(ret, 0, "Escaped empty string");
+	zassert_equal(len, 0, "Length of empty escaped string is zero");
+	zassert_equal(empty[0], '\0', "Empty string remains empty");
+}
+
+static void test_json_escape_no_op(void)
+{
+	char nothing_to_escape[] = "hello,world:!";
+	const char *expected = "hello,world:!";
+	size_t len = sizeof(nothing_to_escape) - 1;
+	ssize_t ret;
+
+	ret = json_escape(nothing_to_escape, &len, sizeof(nothing_to_escape));
+	zassert_equal(ret, 0, "Nothing to escape handled correctly");
+	zassert_equal(len, sizeof(nothing_to_escape) - 1,
+		      "Didn't change length of already escaped string");
+	zassert_true(!strcmp(nothing_to_escape, expected),
+		     "Didn't alter string with nothing to escape");
+}
+
+static void test_json_escape_bounds_check(void)
+{
+	char not_enough_memory[] = "\tfoo";
+	size_t len = sizeof(not_enough_memory) - 1;
+	ssize_t ret;
+
+	ret = json_escape(not_enough_memory, &len, sizeof(not_enough_memory));
+	zassert_equal(ret, -ENOMEM, "Bounds check OK");
+}
+
 void test_main(void)
 {
 	ztest_test_suite(lib_json_test,
@@ -180,7 +265,12 @@ void test_main(void)
 			 ztest_unit_test(test_json_missing_quote),
 			 ztest_unit_test(test_json_wrong_token),
 			 ztest_unit_test(test_json_item_wrong_type),
-			 ztest_unit_test(test_json_key_not_in_descr)
+			 ztest_unit_test(test_json_key_not_in_descr),
+			 ztest_unit_test(test_json_escape),
+			 ztest_unit_test(test_json_escape_one),
+			 ztest_unit_test(test_json_escape_empty),
+			 ztest_unit_test(test_json_escape_no_op),
+			 ztest_unit_test(test_json_escape_bounds_check)
 			 );
 
 	ztest_run_test_suite(lib_json_test);