ISO-2022-JP does not reset state when returning error

[annevk]

What is the issue with the Encoding Standard?

As noted in web-platform-tests/wpt#56892 by @ChALkeR when you stream ISO-2022-JP content in fatal mode you can get an exception and then after that exception it'll continue in the same state where it was before the exception.

Other encodings reset state when returning error.

I think this was intentional because of the non-fatal mode (should probably double check), but maybe for the fatal mode we should do something in the API. E.g., we could reset the internal decoder before throwing.

[ChALkeR]

My take is continuing streaming after a fatal mode has thrown an error is a very bad idea, regardless of the encoding, and should be just forbidden for all encodings.

Demo on utf-16:

const x = Uint8Array.of(1, 0xd8, 42, 0, 42, 0, 43, 0, 47, 0)
for (const fatal of [false, true]) {
  console.log({ fatal })
  const d = new TextDecoder('utf-16le', { fatal })
  const chunk = (arr, stream) => {
    try {
      console.log(escape(d.decode(arr, { stream })))
    } catch {
      console.log('ERR')
    }
  }

  chunk(x.subarray(0, 7), true)
  chunk(x.subarray(7, 9), true) // have we reset state before this call?
  chunk(x.subarray(9))
}

Results in Chrome and Firefox/WebKit/Deno/Servo are different. And I would argue and both actually make no sense

Chrome does not reset the alignment of bytes, everything else does

But that reset makes decoding results differ from non-fatal mode on the same data
Resetting doesn't return a chunk of the data for some input chunks and errors for some chunks, it returns different data for the chunks that did not error

Same will happen for multi-byte encodings, as they contain state

---

Even on single-byte encodings, splitting fatal stream into different chunks looses different data
Decoding should not be dependent on the shape of the chunks, and fatal streaming inherently breaks that

[annevk]

Yeah, always resetting the internal decoder in the API before throwing makes a lot of sense to me. I don't think we have to change the encodings themselves.

@ricea @hsivonen any objections to that?

[ChALkeR]

@annevk resetting does not solve the issue and will still be inconsistent.

The only consistent behavior that I could think of that allows combining fatal: true and stream: true is setting an errored flag and continuing throwing on any input until a final non-stream decode() clears the flag (and also throws one final time).

That way, no misdecoded data will produced, and the streamed output will always match a non-fatal prefix.

E.g. resetting an ISO-2022-JP decoder to ASCII and mismatching non-replaced data between replacement and fatal, or mismatching non-replaced utf-16 data between replacement and fatal could pose security risks.

With auto-resets, both ISO-2022-JP and UTF-16 could be tricked into outputting html code after a reset even when a replacement-decoded text does not contain any html syntax.

ISO-2022-JP could be tricked into that without resets, too.

[ChALkeR]

E.g.:

const x = Uint8Array.fromHex('00d800d80101003c007300630072006900700074003e0061006c006500720074002800310029003c002f007300630072006900700074003e0000')
for (const fatal of [false, true]) {
  console.log({ fatal })
  const d = new TextDecoder('utf-16le', { fatal })
  const chunk = (arr, stream) => {
    try {
      console.log(d.decode(arr, { stream }))
    } catch {
      console.log('ERR')
    }
  }

  chunk(x.subarray(0, 7), true) // ignores the error and continues
  chunk(x.subarray(7, -1), true) // have we reset state/queue before this call?
  chunk(x.subarray(-1))
}

(Behaves differently in Chrome and the rest)