RFR: 8194154 patch for crash at File.getCanonicalPath()

[yumin qi]

HI, Roger and Brian

  Updated again in same link.
  I could not run jtreg so please have a look if the options are good for
@run

Thanks
Yumin

On Wed, Feb 14, 2018 at 11:13 AM, Roger Riggs <Roger.Riggs at oracle.com>
wrote:

> Hi,
>
> In the test, the message can be improved; though the exception should
> never occur
> it will be misleading if it does.
> Instead:
>     "Changing property user.dir should have no effect on the
> getCanonicalPath"
>
> It is cleaner to throw AssertionError for the test failure.
> Throwing FileSystemException makes it look like it came from inside the
> file system implementation.
>
> Regards, Roger
>
>
> On 2/13/2018 6:45 PM, yumin qi wrote:
>
>> Alan,
>>
>>    In fact, if property "user.dir" is cached, the problem will go away
>> without any changes to native or UnixFileSystem.java, since it won't
>> canonicalize the string supplied from user. The output will be
>>    getProperty("user.dir") + "/" + file. (The result is not as expected,
>> user can not change the property, means user has to work around the
>> problem
>> to reach their targeted directory to do things like loading classes)
>>
>>    Any change to "user.dir" in java program does not have any effect.
>>    For Windows:
>>
>> --- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java Fri Feb
>> 02
>> 10:32:59 2018 -0800
>> +++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java Tue Feb
>> 13
>> 23:40:21 2018 +0000
>> @@ -43,12 +43,14 @@
>>       private final char slash;
>>       private final char altSlash;
>>       private final char semicolon;
>> +    private final String userDir;
>>
>>       public WinNTFileSystem() {
>>           Properties props = GetPropertyAction.privilegedGetProperties();
>>           slash = props.getProperty("file.separator").charAt(0);
>>           semicolon = props.getProperty("path.separator").charAt(0);
>>           altSlash = (this.slash == '\\') ? '/' : '\\';
>> +        userDir = props.getProperty("user.dir");
>>       }
>>
>>       private boolean isSlash(char c) {
>> @@ -347,7 +349,7 @@
>>       private String getUserPath() {
>>           /* For both compatibility and security,
>>              we must look this up every time */
>> -        return normalize(System.getProperty("user.dir"));
>> +        return normalize(userDir);
>>       }
>>
>>       private String getDrive(String path) {
>>
>>    Does it need normalize(userDir) in getUserPath()? I think we need here.
>>
>>    I will update the webrev and send for another round of review: the
>> change
>> in native will be reverted, and made changes to UnixFileSystem.java.
>>
>>
>> Thanks
>> Yumin
>>
>>
>> On Mon, Feb 12, 2018 at 3:09 AM, Alan Bateman <Alan.Bateman at oracle.com>
>> wrote:
>>
>> On 09/02/2018 18:01, Alan Bateman wrote:
>>>
>>> :
>>>>
>>>> I'll study the patch you have but I think we also need to create issues
>>>> to get us to the point where changing this system property in a running
>>>> VM
>>>> doesn't impact running code.
>>>>
>>>> Looking at it again, I think we should change java.io.UnixFileSystem
>>> (and
>>> the Windows equivalent) to cache the value of user.dir to avoid difficult
>>> to diagnose issues with bad code changing the value of this property in a
>>> running VM. This should reduce the issue down to cases where user.dir is
>>> changed on the command line (never supported either of course) to a value
>>> that is not "/" but has trailing or duplicate slashes.
>>>
>>> When reduced down then the alternatives are to change the native
>>> canonicalize method as you have done or alternatively do it once at
>>> UnixFileSystem initialization time so that canonicalize does not have to
>>> deal with this case. The former would require changing the description of
>>> the function (it currently reads "The input path is assumed to contain no
>>> duplicate slashes"), the latter avoids any changes to the native
>>> implementation.
>>>
>>> -Alan
>>>
>>>
>>> diff -r 0937e5f799df src/java.base/unix/classes/jav
>>> a/io/UnixFileSystem.java
>>> --- a/src/java.base/unix/classes/java/io/UnixFileSystem.java    Sat Feb
>>> 10 07:06:16 2018 -0500
>>> +++ b/src/java.base/unix/classes/java/io/UnixFileSystem.java    Mon Feb
>>> 12 10:49:40 2018 +0000
>>> @@ -34,12 +34,14 @@
>>>       private final char slash;
>>>       private final char colon;
>>>       private final String javaHome;
>>> +    private final String userDir;
>>>
>>>       public UnixFileSystem() {
>>>           Properties props = GetPropertyAction.privilegedGe
>>> tProperties();
>>>           slash = props.getProperty("file.separator").charAt(0);
>>>           colon = props.getProperty("path.separator").charAt(0);
>>>           javaHome = props.getProperty("java.home");
>>> +        userDir = props.getProperty("user.dir");
>>>       }
>>>
>>>
>>> @@ -128,7 +130,11 @@
>>>
>>>       public String resolve(File f) {
>>>           if (isAbsolute(f)) return f.getPath();
>>> -        return resolve(System.getProperty("user.dir"), f.getPath());
>>> +        SecurityManager sm = System.getSecurityManager();
>>> +        if (sm != null) {
>>> +            sm.checkPropertyAccess("user.dir");
>>> +        }
>>> +        return resolve(userDir, f.getPath());
>>>       }
>>>
>>>       // Caches for canonicalization results to improve startup
>>> performance.
>>>
>>>
>>>
>