On Wed, 18 Jun 2025 13:11:00 GMT, Alan Bateman <al...@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). > > src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java > line 56: > >> 54: // about percent encoding. However, we choose to treat the module >> name is if >> 55: // it were a URL authority (since Java package/module names are >> historically >> 56: // strongly associated with internet domain names). > > "However, we choose to treat the module name is if URL authority" - I don't > think we should be put in the comment as it is confusing to suggest the URL > authority component. The URL scheme documented in JEP 220 always put the > module and resource name in the URL path component. It's just historical > that it allowed for encoding of the resource name, wasn't given enough > thought in JDK 9 when this url stream handler was added. Happy to accept rewording here. I do want to pull out that there *is* a conceptual reason for treating module names like domain authorities though, or just make the code treat the whole path the same. Having unexplained weirdness like this just ends up being a drain on future maintainers otherwise. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25871#discussion_r2154596891
