On Fri, 20 Jun 2025 11:13:12 GMT, David Beaumont <d...@openjdk.org> wrote:
>> Simplifying JavaRuntimeURLConnection to avoid accidentally returning
>> non-resource data to users.
>>
>> This change has the following distinct parts:
>> 1. Refactor code to use Node instead of directly accessing low level
>> ImageLocation type.
>> 2. Remove unnecessary use of "Resource" interface and related URL generation
>> code (completely unreachable).
>> 3. Adding comments explaining why there's a non-obvious distinction in how
>> module and resource names are treated with respect to URL percent encoding.
>> 4. Small constructor logic simplification (module name cannot be null
>> anymore)
>> 5. Small simplification around 'READER' use, since it is impossible for that
>> to ever be null (other users of ImageReaderFactory already assume it could
>> never be null, and code path analysis agrees).
>> 6. Adding tests for the non-resource cases.
>> 7. Adding extra test data to check the behaviour with respect to things like
>> percent escaping (previously untested).
>> 8. Adding TODO comments for things I could do in this PR or later (reviewer
>> opinions welcome).
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Feedback changes and renaming field to match nomenclature of JEP 220.
src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java
line 128:
> 126: if (path.indexOf('%') == -1) {
> 127: // Nothing to decode (overwhelmingly common case).
> 128: return path;
Does this make a performance difference, checking for '%' is the first thing in
ParseUtil.decode.
It seems redundant to check here too.
src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java
line 133:
> 131: return ParseUtil.decode(path);
> 132: } catch (IllegalArgumentException e) {
> 133: throw new MalformedURLException(e.getMessage());
The old code treated this as fatal, throwing InternalError.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25871#discussion_r2159338022
PR Review Comment: https://git.openjdk.org/jdk/pull/25871#discussion_r2159344192
