RFR: 8311216: DataURI can lose information in some charset environments [v4]

[Andy Goryachev]

On Sat, 28 Oct 2023 20:21:55 GMT, Michael Strauß <mstrauss at 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 angorya (Reviewer).

modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 284:

> 282:         return digit;
> 283:     }
> 284: 

very minor: could we avoid this empty line please?

modules/javafx.graphics/src/test/java/test/com/sun/javafx/util/DataURITest.java line 218:

> 216:         assertTrue(ex.getMessage().startsWith("Incomplete"));
> 217:     }
> 218: 

very minor: could we avoid this empty line please?
(i'll re-approve if you choose to fix this)

-------------

PR Review: https://git.openjdk.org/jfx/pull/1165#pullrequestreview-1704378683
PR Review Comment: https://git.openjdk.org/jfx/pull/1165#discussion_r1376370617
PR Review Comment: https://git.openjdk.org/jfx/pull/1165#discussion_r1376371958