Slurping corrupts multibyte characters that cross a 4095-byte boundary

[claui]

To Reproduce

Slurp a file that contains a multibyte character that crosses a 4095-byte boundary.

For example, a text encoded in UTF-8 that is 4096 bytes long and ends with the ü character (U+00FC, 0xc3 0xbc in UTF-8):

# Example text encoded in UTF-8, 4096 bytes long;
# ends with a multibyte character that crosses the 4095-byte boundary
{ yes '*' | tr -d $'\n' | head -c "$(( 4095 - 1 ))"; LC_ALL=en_US.UTF-8 printf '\u00fc'; } | xxd -g 1

When slurping such a file, jq replaces that character with two instances of the Unicode replacement character (U+FFFD, 0xef 0xbf 0xbd in UTF-8) as if it had broken up the character at the 4095-byte boundary:

# Feeding the example text into jq with slurping corrupts the `ü`
LC_ALL=en_US.UTF-8 jq -rsR . <(yes '*' | tr -d $'\n' | head -c "$(( 4095 - 1 ))"; LC_ALL=en_US.UTF-8 printf '\u00fc') | xxd -g 1

Expected behavior

I would expect that, when slurping, jq keeps multibyte characters intact, just like it does when I use --rawfile with no slurping:

# Feeding the same example text into jq using `--rawfile` with no slurping processes the `ü` correctly
LC_ALL=en_US.UTF-8 jq -nr --rawfile payload <(yes '*' | tr -d $'\n' | head -c "$(( 4095 - 1 ))"; LC_ALL=en_US.UTF-8 printf '\u00fc') '$payload' | xxd -g 1

Environment (please complete the following information):

• OS and Version: Arch Linux
• jq v1.8.1

Additional context

• This behavior doesn’t depend on what I tell jq to do with the input once it’s been slurped.
  For example, I can omit -r and it still breaks; I can wrap it inside an object (e.g. { foo: . }) and it still breaks.

• The same corruption occurs with any integer multiple of 4095. You can replace the 4095 in the above expression with e.g. 2 * 4095 (= 8189), 3 * 4095 (= 12284), 4 * 4095 (= 16379), and so on.

• The command line in the example uses Bash’s process substitution just for brevity; you can use an actual file instead (or even a different shell) but the issue remains.

• I couldn’t reproduce the issue using --rawfile; it only seems to occur when slurping is involved (-sR).

• The issue no longer occurs once you increment or decrement the - 1 offset in the command line so the character no longer straddles the 4095-byte boundary.

• I haven’t looked at the implementation of the slurping feature yet. From the outside, it smells a little like a buffer of 4096 bytes (including a null terminator?) might be involved, and the bytes from the buffer might be decoded into text without accounting for incomplete multibyte characters at either buffer boundary.

[claui]

Just noticed that depending on the input file, the corruption may occur at boundaries other than 4095.

For the reproducer given, it’s always an integer multiple of 4095, though.

[wader]

Hi, thanks for the report. That is interesting, so it's more about boundary between files than a buffer?

$ jq -rRs . <(echo -en '\xc3') <(echo -en '\xb6') | hexdump -C
00000000  ef bf bd ef bf bd 0a                              |.......|
00000007
$ jq -rRs . <(echo -en '\xc3\xb6') | hexdump -C
00000000  c3 b6 0a                                          |...|
00000003

Interestingly same does not happen when in "normal" json mode (that a bit weirdly can have values spanning files):

$ jq -r . <(echo -en '"\xc3') <(echo -en '\xb6"') | hexdump -C
00000000  c3 b6 0a                                          |...|
00000003
$ jq -r . <(echo -en '"\xc3\xb6"') | hexdump -C
00000000  c3 b6 0a                                          |...|
00000003

[claui]

so it's more about boundary between files than a buffer?

Hmm, when jq wants to read from a file, then chances are it has to send not just one read request to the OS, but two or more.
Could it be that the boundary of each chunk that results from an individual read request might trigger the bug?

I think it would be consistent with both your example and mine:

• Your example triggers the bug because it forces jq to issue two separate read operations: one per file handle.

• My example triggers the bug because the file may be too large to fit in the read buffer that jq uses, which would cause jq to issue two separate read operations: one that fills the entire buffer, and another one that returns the rest of the file.

For each chunk of bytes that it receives from the OS, could it be that jq attempts to convert each chunk to a string on its own (instead of treating the chunks as a coherent stream)? That might explain why both your and my examples trigger the bug.

[claui]

Root cause

Slurping can involve raw input or non-raw input.
I haven’t looked at the non-raw input case yet. It might have the same bug but it’s not immediately clear to me how it works.

For raw input, here’s one of the lines where slurping breaks:

jq/src/util.c

Line 408 in a8115e5

state->slurped = jv_string_concat(state->slurped, jv_string_sized(state->buf, state->buf_valid_len));

The problem is that jv_string_sized detects invalid UTF-8 and replaces those broken-up characters with U+FFFD.
(I don’t blame it for that. Expecting a jv_string to contain valid UTF-8 at all times is a healthy invariant, I guess.)

There’s a total of three problematic invocations in that block.

Why it works with --rawfile and positional parameters

Here’s the reason why neither the --rawfile method nor files as positional parameters trigger the bug:

jq/src/jv_file.c

Lines 25 to 27 in e84d171

	// To avoid mangling UTF-8 multi-byte sequences that cross the end of our read
	// buffer, we need to be able to read the remainder of a sequence and add that
	// before appending.

Turns out that the same issue was already known and fixed back in 2017 via e84d171!
But only for regular file reading, not for slurping. Slurping takes a whole different code path, and I assume it does that for a reason.

Wouldn’t the jv_utf8_backtrack mechanism used in jv_load_file also be helpful for the slurping case?

See also #1317 and #1696.

[wader]

Ah now i see, yeap both cases seem to be a problem for raw slurp. Haven't look closer yet either but as you say using a jv_string like that wont work. Nice find about the rawfile fix, hmm so that is why jvp_utf8_backtrack was added :) i used it when adding some of the trim builtins.

Another way to reproduce the buffer boundary issues:

$ jq -rn '"😱"*5000' | jq -Rs
<...cut...> 😱😱😱���😱😱😱😱😱😱 <...cut...>

[wader]

Poked around a bit, it seems like raw and non-raw input share reading logic (jq_util_input_read_more) that takes care of some line handling that complicates things. I wonder if it would make sense to do raw slurp separately as it don't need line number counting etc, then maybe it could also solve this the same way as ---rawfile.

[itchyny]

Fix like this? Not sure what happens if the input terminates with invalid characters.

diff --git a/src/util.c b/src/util.c
index bcb86da..41ee4b4 100644
--- a/src/util.c
+++ b/src/util.c
@@ -65,6 +65,7 @@ void *alloca (size_t);
 #include "util.h"
 #include "jq.h"
 #include "jv_alloc.h"
+#include "jv_unicode.h"
 
 #ifdef WIN32
 FILE *fopen(const char *fname, const char *mode) {
@@ -296,7 +297,9 @@ static int jq_util_input_read_more(jq_util_input_state *state) {
     char *res;
     memset(state->buf, 0xff, sizeof(state->buf));
 
-    while (!(res = fgets(state->buf, sizeof(state->buf), state->current_input)) &&
+    const int max_utf8_len = 4;
+    const int max_gets_len = sizeof(state->buf) - max_utf8_len;
+    while (!(res = fgets(state->buf, max_gets_len, state->current_input)) &&
            ferror(state->current_input) && errno == EINTR)
       clearerr(state->current_input);
     if (res == NULL) {
@@ -304,7 +307,7 @@ static int jq_util_input_read_more(jq_util_input_state *state) {
       if (ferror(state->current_input))
         state->failures++;
     } else {
-      const char *p = memchr(state->buf, '\n', sizeof(state->buf));
+      const char *p = memchr(state->buf, '\n', max_gets_len);
 
       if (p != NULL)
         state->current_line++;
@@ -330,13 +333,18 @@ static int jq_util_input_read_more(jq_util_input_state *state) {
          * terminating '\0'. This only works because we previously memset our
          * buffer with something nonzero.
          */
-        for (i = sizeof(state->buf) - 1; i > 0; i--) {
+        for (i = max_gets_len - 1; i > 0; i--) {
           if (state->buf[i] == '\0')
             break;
         }
         state->buf_valid_len = i;
       } else if (p == NULL) {
-        state->buf_valid_len = sizeof(state->buf) - 1;
+        state->buf_valid_len = max_gets_len - 1;
+        char *end = state->buf + state->buf_valid_len;
+        int len = 0;
+        if (jvp_utf8_backtrack(end-1, end-max_utf8_len, &len) && len > 0) {
+          state->buf_valid_len += fread(end, 1, len, state->current_input);
+        }
       } else {
         state->buf_valid_len = (p - state->buf) + 1;
       }

[itchyny]

testing one-liner...

for ((i=1; i<5; i++)); do for ((j=1000; j<1200; j++)); do jq -rn --argjson i $i --argjson j $j '"a"*$i+"𩸽"*$j' | ./jq -Rse 'explode | unique | implode == "\na𩸽"' >/dev/null || { echo "$i, $j failed"; break }; done; done