On Sat, 28 Oct 2023 18:11:08 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> DataURI uses the following implementation to decode the percent-encoded
>> payload of a "data" URI:
>>
>>
>> ...
>> String data = uri.substring(dataSeparator + 1);
>> Charset charset = Charset.defaultCharset();
>> ...
>> URLDecoder.decode(data.replace("+", "%2B"), charset).getBytes(charset)
>>
>>
>> This approach only works if the charset that is passed into
>> `URLDecoder.decode` and `String.getBytes` doesn't lose information when
>> converting between `String` and `byte[]` representations, as might happen in
>> a US-ASCII environment.
>>
>> This PR solves the problem by not using `URLDecoder`, but instead simply
>> decoding percent-encoded escape sequences as specified by RFC 3986, page 11.
>>
>> **Note to reviewers**: the failing test can only be observed when the JVM
>> uses a default charset that can't represent the payload, which can be
>> enforced by specifying the `-Dfile.encoding=US-ASCII` VM option.
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> review changes
Marked as reviewed by jhendrikx (Committer).
modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 211:
> 209: private static byte[] decodePercentEncoding(String input) {
> 210: try (var output = new ByteArrayOutputStream(size(input))) {
> 211: decodePercentEncodingToStream(input, output);
Thanks for making this change, I think it is a good improvement.
minor: The `ByteArrayOutputStream` and its not so useful `IOException`s can now
be eliminated by having `decodePercentEncodingToStream` accepting a 2nd
parameter size, and having it create and return a `byte[]` directly.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1165#pullrequestreview-1702847680
PR Review Comment: https://git.openjdk.org/jfx/pull/1165#discussion_r1375315507
