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

[Michael Strauß]

On Mon, 30 Oct 2023 15:13:26 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review changes
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 221:
> 
>> 219: 
>> 220:         ExpectedCharacter expectedCharacter = ExpectedCharacter.DEFAULT;
>> 221:         byte[] output = new byte[computePayloadSize(input)];
> 
> just a thought: what if the url contains no %?  in this case one can simply return the input string.
> I think we could return -1 from computePayloadSize() to indicate that condition.

The return value is not a string, it's a byte array. So we need to do the conversion anyway, which is what this method is doing quite efficiently by now. It's probably not useful to use another conversion method instead, at least not in terms of performance.

> 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)

I haven't been able to find out whether there's a recommended style in OpenJFX with regards to empty lines at the end of a class declaration. The codebase is very inconsistent about that...

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1165#discussion_r1376670267
PR Review Comment: https://git.openjdk.org/jfx/pull/1165#discussion_r1376672461