PercentCodec decoder to disallow non-ASCII characters in input - #691
Conversation
arturobernalg
left a comment
There was a problem hiding this comment.
I am afraid this implementation changes the existing behavior in two problematic ways.
CharsetDecoder#newDecoder() defaults to REPORT, while flushBin ignores the returned CoderResult. Unconsumed bytes are then retained by compact() and can be combined with a later percent-encoded sequence across a literal character. For example, decoding %C3x%A4 as UTF-8 currently produces \uFFFDx\uFFFD, whereas this change produces xä.
The encode fast path also bypasses charset encoding for the initial safe-character sequence. For example, encoding abc as UTF-16 currently produces %FE%FF%00a%00b%00c, whereas this change produces abc.
I think both cases need to be addressed before this can be merged.
void testDecodeDoesNotCombineEscapedBytesAcrossLiteralCharacters() {
assertEquals(
"\uFFFDx\uFFFD",
PercentCodec.decode("%C3x%A4", StandardCharsets.UTF_8));
}
@Test
void testEncodeHonorsUtf16Charset() {
assertEquals(
"%FE%FF%00a%00b%00c",
PercentCodec.encode("abc", StandardCharsets.UTF_16));
}```
|
@arturobernalg Thank you so much for a thorough review. I will re-design the whole thing and will likely take a different, simpler approach. |
4ef802c to
467b460
Compare
467b460 to
9259715
Compare
|
@arturobernalg I dropped all attempts at memory optimization and made the Please do another pass. |
@arturobernalg Could you please double-check?