Re: RFR: 8290499: new File(parent, "/") breaks normalization – creates File with slash at the end

2023-05-26 Thread Roger Riggs 
On Tue, 23 May 2023 22:49:57 GMT, Brian Burkhalter  wrote:

> In `java.io.File`, change the constructors `File(File,String)` and 
> `File(String,String)` so that they do not for typical cases return a `File` 
> whose path has a trailing name separator.

LGTM

-

Marked as reviewed by rriggs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14109#pullrequestreview-1446514781

Re: RFR: 8290499: new File(parent, \"/\") breaks normalization – creates File with slash at the end

2023-05-24 Thread Brian Burkhalter 
On Wed, 24 May 2023 15:25:37 GMT, Roger Riggs  wrote:

>> In `java.io.File`, change the constructors `File(File,String)` and 
>> `File(String,String)` so that they do not for typical cases return a `File` 
>> whose path has a trailing name separator.
>
> src/java.base/unix/classes/java/io/UnixFileSystem.java line 109:
> 
>> 107: if (len > 1 && s.charAt(len - 1) == '/')
>> 108: return s.substring(0, len - 1);
>> 109: return s;
> 
> If there are multiple trailing `/`, is it sufficient to remove only a single 
> `/`?
> Does this have the same result as `normalize(pathname)`?

The main calling context is

if (parent.path.isEmpty()) {
this.path = FS.resolve(FS.getDefaultParent(),
   FS.normalize(child));
} else {
this.path = FS.resolve(parent.path,
   FS.normalize(child));
}

where `parent` is non-`null` and already normalized. Thus both components 
should already be normalized and multiple trailing `/` impossible. I _think_ 
that it should give the same result as `normalize(pathname)`.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14109#discussion_r1204421403

Re: RFR: 8290499: new File(parent, \"/\") breaks normalization – creates File with slash at the end

2023-05-24 Thread Roger Riggs 
On Tue, 23 May 2023 22:49:57 GMT, Brian Burkhalter  wrote:

> In `java.io.File`, change the constructors `File(File,String)` and 
> `File(String,String)` so that they do not for typical cases return a `File` 
> whose path has a trailing name separator.

src/java.base/unix/classes/java/io/UnixFileSystem.java line 109:

> 107: if (len > 1 && s.charAt(len - 1) == '/')
> 108: return s.substring(0, len - 1);
> 109: return s;

If there are multiple trailing `/`, is it sufficient to remove only a single 
`/`?
Does this have the same result as `normalize(pathname)`?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14109#discussion_r1204387220

RFR: 8290499: new File(parent, \"/\") breaks normalization – creates File with slash at the end

2023-05-23 Thread Brian Burkhalter 
In `java.io.File`, change the constructors `File(File,String)` and 
`File(String,String)` so that they do not for typical cases return a `File` 
whose path has a trailing name separator.

-

Commit messages:
 - 8290499: new File(parent, \"/\") breaks normalization – creates File with 
slash at the end

Changes: https://git.openjdk.org/jdk/pull/14109/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=14109=00
  Issue: https://bugs.openjdk.org/browse/JDK-8290499
  Stats: 34 lines in 3 files changed: 28 ins; 0 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/14109.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14109/head:pull/14109

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