>From 9a5c6a9eab6cb5ddbbb1f1c0940a561f6895b478 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 4 Oct 2017 18:38:07 -0700 Subject: [PATCH] Minor JSON cleanups, mostly for overflow * src/json.c (json_has_prefix): Simplify via strncmp. (json_has_suffix): Indent a la GNU. (json_make_string): Take size_t, not ptrdiff_t, and check its range. Simplify callers accordingly. (json_out_of_memory, json_to_lisp): Just call memory_full. (check_string_without_embedded_nulls): Use strlen, not memchr; it is typically faster. (lisp_to_json_toplevel_1, json_to_lisp): Do not bother with _GL_ARG_NONNULL on static functions; it is not worth the trouble. (lisp_to_json_toplevel_1, lisp_to_json, json_read_buffer_callback) (define_error): Remove useless checks. (json_insert): Just call buffer_overflow. (json_to_lisp): Just signal overflow error, to be consistent with other signalers. Use allocate_vector instead of Fmake_vector, to avoid need for initializing vector twice. Use make_hash_table instead of Fmake_hash_table, as it is a bit simpler. --- src/json.c | 93 +++++++++++++++++++++----------------------------------------- 1 file changed, 32 insertions(+), 61 deletions(-) diff --git a/src/json.c b/src/json.c index 79be55bd54..5138315da1 100644 --- a/src/json.c +++ b/src/json.c @@ -31,9 +31,7 @@ along with GNU Emacs. If not, see . */ static bool json_has_prefix (const char *string, const char *prefix) { - size_t string_len = strlen (string); - size_t prefix_len = strlen (prefix); - return string_len >= prefix_len && memcmp (string, prefix, prefix_len) == 0; + return strncmp (string, prefix, strlen (prefix)) == 0; } static bool @@ -41,22 +39,23 @@ json_has_suffix (const char *string, const char *suffix) { size_t string_len = strlen (string); size_t suffix_len = strlen (suffix); - return string_len >= suffix_len - && memcmp (string + string_len - suffix_len, suffix, suffix_len) == 0; + return (string_len >= suffix_len + && (memcmp (string + string_len - suffix_len, suffix, suffix_len) + == 0)); } static Lisp_Object -json_make_string (const char *data, ptrdiff_t size) +json_make_string (const char *data, size_t size) { + if (PTRDIFF_MAX < size) + string_overflow (); return make_specified_string (data, -1, size, true); } static Lisp_Object json_build_string (const char *data) { - size_t size = strlen (data); - eassert (size < PTRDIFF_MAX); - return json_make_string (data, size); + return json_make_string (data, strlen (data)); } static Lisp_Object @@ -68,7 +67,7 @@ json_encode (Lisp_Object string) static _Noreturn void json_out_of_memory (void) { - xsignal0 (Qjson_out_of_memory); + memory_full (SIZE_MAX); } static _Noreturn void @@ -97,7 +96,7 @@ static void check_string_without_embedded_nulls (Lisp_Object object) { CHECK_STRING (object); - CHECK_TYPE (memchr (SDATA (object), '\0', SBYTES (object)) == NULL, + CHECK_TYPE (strlen (SSDATA (object)) == SBYTES (object), Qstring_without_embedded_nulls_p, object); } @@ -114,15 +113,12 @@ static json_t *lisp_to_json (Lisp_Object) ATTRIBUTE_WARN_UNUSED_RESULT; /* This returns Lisp_Object so we can use unbind_to. The return value is always nil. */ -static _GL_ARG_NONNULL ((2)) Lisp_Object +static Lisp_Object lisp_to_json_toplevel_1 (Lisp_Object lisp, json_t **json) { if (VECTORP (lisp)) { ptrdiff_t size = ASIZE (lisp); - eassert (size >= 0); - if (size > SIZE_MAX) - xsignal1 (Qoverflow_error, build_string ("vector is too long")); *json = json_check (json_array ()); ptrdiff_t count = SPECPDL_INDEX (); record_unwind_protect_ptr (json_release_object, json); @@ -194,9 +190,6 @@ lisp_to_json (Lisp_Object lisp) { Lisp_Object encoded = json_encode (lisp); ptrdiff_t size = SBYTES (encoded); - eassert (size >= 0); - if (size > SIZE_MAX) - xsignal1 (Qoverflow_error, build_string ("string is too long")); return json_check (json_stringn (SSDATA (encoded), size)); } @@ -239,7 +232,7 @@ json_insert (void *data) { const struct json_buffer_and_size *buffer_and_size = data; if (buffer_and_size->size > PTRDIFF_MAX) - xsignal1 (Qoverflow_error, build_string ("buffer too large")); + buffer_overflow (); insert (buffer_and_size->buffer, buffer_and_size->size); return Qnil; } @@ -289,7 +282,7 @@ OBJECT. */) return unbind_to (count, Qnil); } -static _GL_ARG_NONNULL ((1)) Lisp_Object +static Lisp_Object json_to_lisp (json_t *json) { switch (json_typeof (json)) @@ -304,32 +297,26 @@ json_to_lisp (json_t *json) { json_int_t value = json_integer_value (json); if (FIXNUM_OVERFLOW_P (value)) - xsignal1 (Qoverflow_error, - build_string ("JSON integer is too large")); + xsignal0 (Qoverflow_error); return make_number (value); } case JSON_REAL: return make_float (json_real_value (json)); case JSON_STRING: - { - size_t size = json_string_length (json); - if (FIXNUM_OVERFLOW_P (size)) - xsignal1 (Qoverflow_error, build_string ("JSON string is too long")); - return json_make_string (json_string_value (json), size); - } + return json_make_string (json_string_value (json), + json_string_length (json)); case JSON_ARRAY: { if (++lisp_eval_depth > max_lisp_eval_depth) xsignal0 (Qjson_object_too_deep); size_t size = json_array_size (json); if (FIXNUM_OVERFLOW_P (size)) - xsignal1 (Qoverflow_error, build_string ("JSON array is too long")); - Lisp_Object result = Fmake_vector (make_natnum (size), Qunbound); + memory_full (size); + struct Lisp_Vector *v = allocate_vector (size); for (ptrdiff_t i = 0; i < size; ++i) - ASET (result, i, - json_to_lisp (json_array_get (json, i))); + v->contents[i] = json_to_lisp (json_array_get (json, i)); --lisp_eval_depth; - return result; + return make_lisp_ptr (v, Lisp_Vectorlike); } case JSON_OBJECT: { @@ -337,10 +324,10 @@ json_to_lisp (json_t *json) xsignal0 (Qjson_object_too_deep); size_t size = json_object_size (json); if (FIXNUM_OVERFLOW_P (size)) - xsignal1 (Qoverflow_error, - build_string ("JSON object has too many elements")); - Lisp_Object result = CALLN (Fmake_hash_table, QCtest, Qequal, - QCsize, make_natnum (size)); + memory_full (size); + Lisp_Object result + = make_hash_table (hashtest_equal, size, DEFAULT_REHASH_SIZE, + DEFAULT_REHASH_THRESHOLD, Qnil, false); struct Lisp_Hash_Table *h = XHASH_TABLE (result); const char *key_str; json_t *value; @@ -399,23 +386,12 @@ json_read_buffer_callback (void *buffer, size_t buflen, void *data) /* First, parse from point to the gap or the end of the accessible portion, whatever is closer. */ ptrdiff_t point = d->point; - ptrdiff_t end; - { - bool overflow = INT_ADD_WRAPV (BUFFER_CEILING_OF (point), 1, &end); - eassert (!overflow); - } - size_t count; - { - bool overflow = INT_SUBTRACT_WRAPV (end, point, &count); - eassert (!overflow); - } + ptrdiff_t end = BUFFER_CEILING_OF (point) + 1; + ptrdiff_t count = end - point; if (buflen < count) count = buflen; memcpy (buffer, BYTE_POS_ADDR (point), count); - { - bool overflow = INT_ADD_WRAPV (d->point, count, &d->point); - eassert (!overflow); - } + d->point += count; return count; } @@ -444,14 +420,11 @@ not moved. */) /* Convert and then move point only if everything succeeded. */ Lisp_Object lisp = json_to_lisp (object); - { - /* Adjust point by how much we just read. Do this here because - tokener->char_offset becomes incorrect below. */ - bool overflow = INT_ADD_WRAPV (point, error.position, &point); - eassert (!overflow); - eassert (point <= ZV_BYTE); - SET_PT_BOTH (BYTE_TO_CHAR (point), point); - } + /* Adjust point by how much we just read. Do this here because + tokener->char_offset becomes incorrect below. */ + eassert (0 <= error.position && error.position <= ZV_BYTE - point); + point += error.position; + SET_PT_BOTH (BYTE_TO_CHAR (point), point); return unbind_to (count, lisp); } @@ -462,8 +435,6 @@ not moved. */) static void define_error (Lisp_Object name, const char *message, Lisp_Object parent) { - eassert (SYMBOLP (name)); - eassert (SYMBOLP (parent)); Lisp_Object parent_conditions = Fget (parent, Qerror_conditions); eassert (CONSP (parent_conditions)); eassert (!NILP (Fmemq (parent, parent_conditions))); -- 2.13.6