RFR: 8298478: (fs) Path.of should allow input to include long path prefix [v2]

Alan Bateman alanb at openjdk.org
Tue Feb 7 11:53:50 UTC 2023


On Tue, 7 Feb 2023 00:23:58 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Remove the long path `\?` or `\?\UNC` prefix before creating the `WindowsPath` instance..
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8298478: Verify parsed type is acceptable for prefix type; add some cases to test

src/java.base/windows/classes/sun/nio/fs/WindowsPathParser.java line 36:

> 34: class WindowsPathParser {
> 35:     private static final String LONG_PATH_PREFIX     = "\\\\?\\";
> 36:     private static final String LONG_PATH_UNC_PREFIX = LONG_PATH_PREFIX + "UNC";

Shouldn't this be "\?\UNC" ?

src/java.base/windows/classes/sun/nio/fs/WindowsPathParser.java line 111:

> 109:             input = input.substring(4);
> 110:         } else {
> 111:             prefixType = PREFIX_TYPE_NONE;

This looks okay but it means 2 x startsWith tests for the common case. If the input doesn't start with LONG_PATH_PREFIX then there is no need to check if starts with the UNC version.

Another idea here is drop the PREFIX_TYPE_xxx fields and just have an expectedType field of type WindowsPathType.

src/java.base/windows/classes/sun/nio/fs/WindowsPathParser.java line 172:

> 170:             throw new InvalidPathException(input, "Long path prefix must precede an absolute path");
> 171:         } else if (prefixType == PREFIX_TYPE_LONG_UNC && type != WindowsPathType.UNC) {
> 172:             throw new InvalidPathException(input, "Long UNC path prefix must precede a UNC path");

I don't think the text should have "must precede" as someone reading this might assume they must use this prefix. Maybe say "cannot be used with ..." ?

test/jdk/java/nio/file/Path/PathOps.java line 1458:

> 1456:         test("\\\\?\\UNC\\server\\share\\dir\\file.dat")      // UNC
> 1457:             .string("\\\\server\\share\\dir\\file.dat");
> 1458:         test("\\\\?\\UNC\\server\\share\\C:\\mnt\\file.dat")  // absolute

Shouldn't this be "\\\?\\UNC\\C:\\file.dat"?

Also can we try a relative path, e.g.  "\\\?\\UNC\\file.dat"

test/jdk/java/nio/file/Path/PathOps.java line 1462:

> 1460:         test("\\\\?\\UNC\\server\\share\\C:file.dat")         // drive-relative
> 1461:             .invalid();
> 1462:         test("\\\\?\\UNC")                                    // empty

Probably want "\\\?\\UNC\") too.

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

PR: https://git.openjdk.org/jdk/pull/12423


More information about the nio-dev mailing list