JSON_EXTRACT and JSON_ARRAY_APPEND re-encode the document as latin1 in a wide character set, and return NULL blaming the user's input

In a character set whose characters are more than one byte wide - ucs2,
utf16, utf16le, utf32 - JSON_EXTRACT() returns NULL for a
document it has just parsed successfully, and complains about a character the
document does not contain:

SET collation_connection='utf16_bin';

SELECT JSON_EXTRACT('{"a":1,"b":2}','$.a');

JSON_EXTRACT('{"a":1,"b":2}','$.a')

NULL

Warnings:

Warning  4036  Character disallowed in JSON in argument 1 to function 'json_extract' at position 2

The document is well formed, JSON_VALID() says so, and every other reading
function handles it. The disallowed character is not in the argument at all.
It is put there by JSON_EXTRACT itself, while copying the value it found
into the result, and the complaint is about the copy.

JSON_ARRAY_APPEND() does the same thing, but only when the path it is
given points at something that is not an array:

SET collation_connection='utf16_bin';

SELECT JSON_ARRAY_APPEND('{"a":[1,2]}','$.a',3) AS target_is_array,

       JSON_ARRAY_APPEND('{"a":1}','$.a',3)     AS target_is_scalar;

target_is_array         target_is_scalar

{"a": [1, 2, 3]}        NULL

Warnings:

Warning  4036  Character disallowed in JSON in argument 1 to function 'json_array_append' at position 14

Same function, same statement, same character set: the branch that appends to
an existing array is correct, and the branch that wraps a non-array value into
one is not.

What the result actually holds

The value is not merely rejected - it is corrupted first, and the rejection is
a consequence. Each byte of the extracted value is read as if it were a
latin1 character and encoded again in the result's character set, so a
value already written two bytes to the character comes out four, with the
original bytes spread across pairs of characters.

For '$.a' above the value is the single character 1, which is
0031 in utf16. The two bytes 00 and 31 are then taken as the
latin1 characters U+0000 and U+0031, and the result holds
00000031 - a NUL followed by a 1, where a single 1 was meant. The
NUL is the disallowed character the warning is about, and position 2 is where
the reading of it stopped.

The width of the reported position follows the width of the character set,
which is the double encoding showing through:

Every form of JSON_EXTRACT is affected

SET collation_connection='utf16_bin';

SELECT HEX(JSON_EXTRACT('{"a":1,"b":2}','$.a'))       AS scalar_value,

       HEX(JSON_EXTRACT('{"a":{"x":1},"b":2}','$.a')) AS object_value,

       HEX(JSON_EXTRACT('{"a":1,"b":2}','$.a','$.b')) AS several_paths;

scalar_value  object_value  several_paths

NULL          NULL          NULL

The rest of the family is correct, which is how narrow the defect is

Measured over the same two-byte character set, on the same server:

JSON_UNQUOTE, JSON_QUOTE and JSON_NORMALIZE hand back a result in
a character set of their own choosing rather than the one asked for. That is
long-standing and unrelated to this report.

Root cause

String::append(const char *, size_t) converts what it is given from
latin1 whenever the string being appended to is one a character cannot fit
into a single byte:

/* sql/sql_string.cc */

bool String::append(const char *s,size_t size)

{

  ...

  /*

    For an ASCII incompatible string, e.g. UCS-2, we need to convert

  */

  if (mbminlen() > 1)

  {

    uint32 add_length= arg_length * mbmaxlen();

    uint dummy_errors;

    if (realloc_with_extra_if_needed(str_length+ add_length))

      return TRUE;

    str_length+= copy_and_convert(Ptr + str_length, add_length, charset(),

                                  s, arg_length, &my_charset_latin1,

                                  &dummy_errors);

    return FALSE;

  }

  ...

}

That is right for text that really is latin1, and it is what makes
str->append('[') put a proper wide bracket into a ucs2 result. It is
wrong for a run of bytes cut out of the document, because those bytes are
already in the document's character set, which is also the result's. They are
encoded a second time.

Three call sites in sql/item_jsonfunc.cc hand it a slice of the document:

/* Item_func_json_extract::read_json - every path that returns a value */

      if (str->append((const char *) value, v_len))

        goto error;

/* Item_func_json_array_append::val_str - the wrap-as-array branch only */

      if (str->append('[') ||

          str->append((const char *) c_from, c_to - c_from) ||

          str->append(", ", 2) ||

          append_json_value(str, args[n_arg+1], &tmp_val, 0, func_name(),

                            (int) n_arg + 1, NULL) ||

          str->append(']') ||

          str->append((const char *) je.s.c_str,

                      js->end() - (const char *) je.s.c_str))

        goto return_null;

The same file copies document slices safely everywhere else, by four different
means: append_simple() and String::q_append() copy the bytes as they
stand, and st_append_json(res, cs, ...) and String::set(ptr, len, cs)
are told which character set the bytes are in. JSON_ARRAY_APPEND's own
array branch, in the arm just above the broken one, uses q_append():

      str->q_append(js->ptr(), ar_end-(const uchar *) js->ptr());

      ...

      str->q_append((const char *) ar_end, str_rest_len);

That is why one branch of the function works and the other does not, and why
JSON_QUERY and JSON_VALUE - which go through
Json_engine_scan::check_and_get_value_scalar() and
check_and_get_value_complex(), both charset-explicit - are unaffected.

Why it surfaces as NULL rather than as visible mojibake

Both functions read their whole result back before returning it, to write it
out in the loose form. That reading is done in the character set the result
claims, so it meets the NUL that the second encoding introduced, stops with
JE_BAD_CHR, and the function returns NULL with warning 4036.

The corruption is therefore never seen. What is seen is a complaint naming
argument 1 and a position inside it, which reads as a statement about the
document the caller supplied. The character it names was inserted by the
server a few lines earlier.

Affected versions

The call in Item_func_json_extract::read_json dates from
27025221fe2 (2016-10-19, "MDEV-9143 JSON_xxx functions"), the original
implementation of the JSON functions, first released in 10.2.0. It has not
been modified since.

MDEV-34143 (0406b2a4ed1, 2024-05-15) rewrote the loop around it, to fix a
crash in Static_binary_string::chop() under exactly these character sets,
and carried the call across unchanged - it appears in the pre-image of that
diff. The same commit added

SET collation_connection='utf16_bin';

SELECT JSON_EXTRACT('{"a": 1,"b": 2}','$.a');

JSON_EXTRACT('{"a": 1,"b": 2}','$.a')

NULL

Warnings:

Warning  4036  Character disallowed in JSON in argument 1 to function 'json_extract' at position 2

to main/func_json.result, so the NULL is currently a recorded expectation.
Neither that report nor its resolution discusses what the function ought to
return under a non-default collation_connection; the report is a crash
report and the fix addresses the crash. The NULL appears to be what the server
did once it stopped crashing, recorded as the new expected output.

All three call sites are present at the tips of 10.5, 10.6, 10.11, 11.4, 11.8,
12.0, 12.1, 12.2, 12.3 and main. Every release from 10.2.0 onward is expected
to be affected.

Verified on 10.11 only (10.11.19-MariaDB-debug). Other branches were read,
not run.

A fourth site exists from 11.4 onwards

Item_func_json_object_to_array, which does not exist in 10.11, copies both
the key and the value of every pair the same way:

/* sql/item_jsonfunc.cc @ upstream/main - convert_to_array() */

        temp_str.append((const char*)key_start, (size_t)(key_end-key_start));

        ...

        temp_str.append((const char *) value, v_len);

Not exercised here, and mentioned only so that whoever works on the versions
that have it knows it is the same defect.

Note

Found while measuring what the JSON functions read, in preparation for an
unrelated change. None of the character sets involved is covered by any
existing test beyond the single MDEV-34143 statement quoted above.