On Mon, 23 Jun 2025 22:31:27 GMT, David Beaumont <[email protected]> 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 three
> additional commits since the last revision:
>
> - Remove redundant fast-path check.
> - Change method name.
> - Feedback changes (maybe some still to be done).
src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java
line 113:
> 111: public long getContentLengthLong() {
> 112: // Note: UncheckedIOException is thrown by the Node subclass in
> 113: // ExplodedImage (this not obvious, so worth calling out).
This protocol handler is for images build, not exploded builds.
ImageReaderFactory.getImageReader will fail if you attempt it with an exploded
build. So I don't think we need this comment or the catch of UIOE.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25871#discussion_r2164237749
