From f7c2e7623c08f30dd9f7e75641b4c3221998488a Mon Sep 17 00:00:00 2001 From: Marti Bolivar Date: Wed, 3 May 2017 15:57:18 -0400 Subject: [PATCH 1/2] lib: json: escape strings in-place Currently, json_escape() allocates a temporary buffer on the stack that's the size of the string being escaped. Stack space is precious and longer JSON strings can run into the hundreds of bytes, so re-implement this routine to escape in place. Signed-off-by: Marti Bolivar --- lib/json/json.c | 96 ++++++++++++++++++++++++------------------------- 1 file changed, 47 insertions(+), 49 deletions(-) diff --git a/lib/json/json.c b/lib/json/json.c index 8b8eef06c176f..41faaf7b3e439 100644 --- a/lib/json/json.c +++ b/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) { From a8039fa98a74baad1641bfdf7df0226a60a98961 Mon Sep 17 00:00:00 2001 From: Marti Bolivar Date: Wed, 3 May 2017 16:33:17 -0400 Subject: [PATCH 2/2] tests: json: add json_escape() tests These all pass. Signed-off-by: Marti Bolivar --- tests/lib/json/src/main.c | 92 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-) diff --git a/tests/lib/json/src/main.c b/tests/lib/json/src/main.c index 945d8ace52bb7..c28df0c994dcc 100644 --- a/tests/lib/json/src/main.c +++ b/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);