Re: JDK 9 RFR of 8148023: File.createTempFile is not adhering to the contract regarding file name lengths

[Brian Burkhalter]

Hi Roger,

I updated the .01 version of the webrev in place. I’ll go ahead and push that.

Thanks,

Brian

On Dec 19, 2016, at 1:24 PM, Roger Riggs  wrote:

> Looks good
> 
> Except for a typo in WinNTFileSystem_md.c:24  "compnent"

Re: JDK 9 RFR of 8148023: File.createTempFile is not adhering to the contract regarding file name lengths

[Roger Riggs]

Hi Brian,

Looks good

Except for a typo in WinNTFileSystem_md.c:24  "compnent"

Regards, Roger


On 12/19/2016 3:54 PM, Brian Burkhalter wrote:

Hi Roger,

Thanks for your suggestions. An updated version of the patch is here:

http://cr.openjdk.java.net/~bpb/8148023/webrev.01/ 



On Dec 19, 2016, at 11:04 AM, Roger Riggs > wrote:



File.java:

- 1906: shortenSubName might reasonably return the new length without
  creating the intermediate string and save on the allocation.


So changed.

- generateFile() line 1922 + --  I assume this slow path is 
infrequently used.
 Otherwise, the computations involving excess could be done 
arithmetically without actually creating the string

 and only create the name when the final length(s) are decided.


Modified.

- line 1952:  appending the generated name to the exception is ok but 
it should omit the directory name;
  its not salient to the error and might expose a sensitive directory 
name.


Updated.


FileSystem.java
- getNameMax;  the length could reasonably be just an int.  Long 
seems a bit excessive
  and raises suspicion that something clever is going on when its 
just a normal value.

 (Though I see the native code uses jlong)


Changed to an int except for the native Unix call. The invoking Java 
code clamps a highly unlikely out-of-range long to Integer.MAX_VALUE.


Thanks,

Brian

Re: JDK 9 RFR of 8148023: File.createTempFile is not adhering to the contract regarding file name lengths

[Brian Burkhalter]

Hi Roger,

Thanks for your suggestions. An updated version of the patch is here:

http://cr.openjdk.java.net/~bpb/8148023/webrev.01/

On Dec 19, 2016, at 11:04 AM, Roger Riggs  wrote:

> File.java:
> 
> - 1906: shortenSubName might reasonably return the new length without
>   creating the intermediate string and save on the allocation.

So changed.

> - generateFile() line 1922 + --  I assume this slow path is infrequently used.
>  Otherwise, the computations involving excess could be done arithmetically 
> without actually creating the string
>  and only create the name when the final length(s) are decided.

Modified.

> - line 1952:  appending the generated name to the exception is ok but it 
> should omit the directory name;
>   its not salient to the error and might expose a sensitive directory name.

Updated.

> FileSystem.java
> - getNameMax;  the length could reasonably be just an int.  Long seems a bit 
> excessive
>   and raises suspicion that something clever is going on when its just a 
> normal value.
>  (Though I see the native code uses jlong)

Changed to an int except for the native Unix call. The invoking Java code 
clamps a highly unlikely out-of-range long to Integer.MAX_VALUE.

Thanks,

Brian

Re: JDK 9 RFR of 8148023: File.createTempFile is not adhering to the contract regarding file name lengths

[Roger Riggs]

Hi Brian,

File.java:

 - 1906: shortenSubName might reasonably return the new length without
   creating the intermediate string and save on the allocation.

 - generateFile() line 1922 + --  I assume this slow path is 
infrequently used.
  Otherwise, the computations involving excess could be done 
arithmetically without actually creating the string

  and only create the name when the final length(s) are decided.

 - line 1952:  appending the generated name to the exception is ok but 
it should omit the directory name;
   its not salient to the error and might expose a sensitive directory 
name.


FileSystem.java
 - getNameMax;  the length could reasonably be just an int.  Long seems 
a bit excessive
   and raises suspicion that something clever is going on when its just 
a normal value.

  (Though I see the native code uses jlong)

$.02, Roger



On 12/16/2016 4:31 PM, Brian Burkhalter wrote:

Please review at your convenience.

Issue:  https://bugs.openjdk.java.net/browse/JDK-8148023
Patch:  http://cr.openjdk.java.net/~bpb/8148023/webrev.00/index.html

The method was not adhering to the specification [1] and was creating long file 
names which caused failures on all platforms. The pertinent portion of the 
specification is:

"To create the new file, the prefix and the suffix may first be adjusted to fit the 
limitations of the underlying platform. If the prefix is too long then it will be 
truncated, but its first three characters will always be preserved. If the suffix is too 
long then it too will be truncated, but if it begins with a period character ('.') then 
the period and the first three characters following it will always be preserved. Once 
these adjustments have been made the name of the new file will be generated by 
concatenating the prefix, five or more internally-generated characters, and the 
suffix."

The code was updated to truncate the filename (leaf) portion of the path name 
to meet the platform file name component length constraints. The IO regression 
tests passed with this change on all platforms.

Thanks,

Brian

[1] 
http://download.java.net/java/jdk9/docs/api/java/io/File.html#createTempFile-java.lang.String-java.lang.String-java.io.File-