Re: RFR: 8287843: File::getCanonicalFile doesn't work for \?\C:\ style paths DOS device paths [v3]

2023-09-20 Thread Brian Burkhalter
On Wed, 20 Sep 2023 18:10:17 GMT, Brian Burkhalter  wrote:

>> In the Windows implementation of java.io.File.getCanonicalPath, strip any 
>> long path or UNC prefix before canonicalizing the remainder of the pathname.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8287843: Move \\?\ prefix stripping to resolve(File)

It might be that the long prefix needs to be stripped local to several 
different methods. Will investigate.

-

PR Comment: https://git.openjdk.org/jdk/pull/15603#issuecomment-1728370663


Re: RFR: 8287843: File::getCanonicalFile doesn't work for \?\C:\ style paths DOS device paths [v3]

2023-09-20 Thread Alan Bateman
On Wed, 20 Sep 2023 18:47:33 GMT, Brian Burkhalter  wrote:

> > File::isAbsolute, it looks like it will return true for input like 
> > `\\\?\\foo` but it will be treated by toAbsolutePath as a relative path.
> 
> Right on:

Ideally the prefix would be removed at the front door, meaning when creating 
the File object, but it may be 20 years too late to do that change. From a 
compatibility perspective then limiting the it to canonicalization should be 
okay but it the path needs to make absolute again after stripping.

-

PR Comment: https://git.openjdk.org/jdk/pull/15603#issuecomment-1728282600


Re: RFR: 8287843: File::getCanonicalFile doesn't work for \?\C:\ style paths DOS device paths [v3]

2023-09-20 Thread Brian Burkhalter
On Wed, 20 Sep 2023 18:42:31 GMT, Alan Bateman  wrote:

> File::isAbsolute, it looks like it will return true for input like 
> `\\\?\\foo` but it will be treated by toAbsolutePath as a relative path.

Right on:

jshell> File f = new File("\\\?\\foo")
f ==> \?\foo

jshell> f.isAbsolute()
$2 ==> true

jshell> f.getAbsolutePath()
$3 ==> "C:\\Users\\bpb\\foo"

jshell> f.getCanonicalPath()
$4 ==> "C:\\Users\\bpb\\foo"

-

PR Comment: https://git.openjdk.org/jdk/pull/15603#issuecomment-1728260652


Re: RFR: 8287843: File::getCanonicalFile doesn't work for \?\C:\ style paths DOS device paths [v3]

2023-09-20 Thread Alan Bateman
On Wed, 20 Sep 2023 18:10:17 GMT, Brian Burkhalter  wrote:

>> In the Windows implementation of java.io.File.getCanonicalPath, strip any 
>> long path or UNC prefix before canonicalizing the remainder of the pathname.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8287843: Move \\?\ prefix stripping to resolve(File)

> So changed in 
> [a8726fb](https://github.com/openjdk/jdk/commit/a8726fbb8a070388f2b9756ee88c746b12c18397)
>  which also adds a couple of test cases. Perhaps some cases should be added 
> to the `GetAbsolutePath` test as well.

Yes, I think it will need tests. The next thing is ask is the behavior of 
File::isAbsolute, it looks like it will return true for input like `\\\?\\foo` 
but it will be treated by toAbsolutePath as a relative path.

-

PR Comment: https://git.openjdk.org/jdk/pull/15603#issuecomment-1728254382


Re: RFR: 8287843: File::getCanonicalFile doesn't work for \?\C:\ style paths DOS device paths [v3]

2023-09-20 Thread Brian Burkhalter
> In the Windows implementation of java.io.File.getCanonicalPath, strip any 
> long path or UNC prefix before canonicalizing the remainder of the pathname.

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8287843: Move \\?\ prefix stripping to resolve(File)

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15603/files
  - new: https://git.openjdk.org/jdk/pull/15603/files/d9d84b8e..a8726fbb

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15603=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=15603=01-02

  Stats: 47 lines in 2 files changed: 24 ins; 14 del; 9 mod
  Patch: https://git.openjdk.org/jdk/pull/15603.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15603/head:pull/15603

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