On Sun, 7 Dec 2025 02:53:54 GMT, Weijun Wang <[email protected]> wrote:
>> I have pushed the changes to proceed without resolution
>> (9f298af59431507a66e3141c54abb59fcf3666f6,
>> 2a012397baf0599e7dbe209975b3b353c3de5617,
>> 1178544bda12bb4a6cd4d4400dad618292f29151,
>> c33bf62c2831acefd90ec476fcfb6d853be873ee).
>>
>> Since we are no longer resolving paths, we can incur in some relative paths
>> complexity, which is perhaps not very friendly in debug message logs. Each
>> relative include can potentially introduce some `../`, which will accumulate
>> if paths are not resolved.
>>
>> So we can end with paths like the following one:
>>
>> /basedir/jdk/conf/security/../../../properties/dir1/../../jdk/conf/security/other.properties
>>
>>
>> Which could be simply logged as:
>>
>> /basedir/jdk/conf/security/other.properties
>>
>>
>> So even when I already adjusted the test case, is perhaps better to undo the
>> test changes and try to beautify the paths in debugging messages (but with
>> `LinkOption.NOFOLLOW_LINKS`, to avoid confusion):
>>
>> diff --git a/src/java.base/share/classes/java/security/Security.java
>> b/src/java.base/share/classes/java/security/Security.java
>> index 36021f42862..533072b0d08 100644
>> --- a/src/java.base/share/classes/java/security/Security.java
>> +++ b/src/java.base/share/classes/java/security/Security.java
>> @@ -311,8 +311,13 @@ private static void loadFromUrl(URL url, LoadingMode
>> mode)
>> private static void debugLoad(boolean start, Object source) {
>> if (sdebug != null) {
>> + if (source instanceof Path path) {
>> + try {
>> + source = path.toRealPath(LinkOption.NOFOLLOW_LINKS);
>> + } catch (IOException ignore) {}
>> + }
>> int level = activePaths.isEmpty() ? 1 : activePaths.size();
>> sdebug.println((start ?
>> ">".repeat(level) + " starting to process " :
>> "<".repeat(level) + " finished processing ") +
>> source);
>> }
>> }
>>
>>
>>
>> NOTE: even with `LinkOption.NOFOLLOW_LINKS`, `path.toRealPath()` fails for
>> the problematic cases, so it would be just a best effort to make the paths
>> clearer for the user.
>>
>> What do you think?
>
> I thought `normalize` will remove those `..` inside?
The problem with
[`Path::normalize`](https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/nio/file/Path.html#normalize())
is the following one:
> Eliminating ".." and a preceding name from a path may result in the path that
> locates a different file than the original path. This can arise when the
> preceding name is a symbolic link.
This can be demonstrated with the following files structure:
/tmp/a
└── /tmp/a/b -> /tmp/x/y
/tmp/x
├── /tmp/x/f2 (name=value)
└── /tmp/x/y
└── /tmp/x/y/f1 (include ../f2)
Which can be created with:
mkdir -p /tmp/x/y /tmp/a
echo 'include ../f2' >/tmp/x/y/f1
echo 'name=value' >/tmp/x/f2
ln -s /tmp/x/y /tmp/a/b
Now let's assume we are processing `/tmp/a/b/f1`, `include ../f2` is computed
as `included`:
jshell -<<'EOF'
Path current = Path.of("/tmp/a/b/f1");
Path included = current.resolveSibling("../f2");
System.out.println(" included: " +
included);
System.out.println(" included.toRealPath(): " +
included.toRealPath());
System.out.println("included.toRealPath(LinkOption.NOFOLLOW_LINKS): " +
included.toRealPath(LinkOption.NOFOLLOW_LINKS));
System.out.println(" included.normalize(): " +
included.normalize());
EOF
Output:
included: /tmp/a/b/../f2
included.toRealPath(): /tmp/x/f2
included.toRealPath(LinkOption.NOFOLLOW_LINKS): /tmp/a/b/../f2
included.normalize(): /tmp/a/f2
But `/tmp/a/f2` doesn't exist:
user@host:~$ cat /tmp/a/f2
cat: /tmp/a/f2: No such file or directory
To destroy the created directories and files use:
rm -rf /tmp/x /tmp/a
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24465#discussion_r2607247993
