[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-swing-dev
Subject:    Re: <Swing Dev> RFR: PIT Bug 8167988 : java.nio.file.InvalidPathException if click button in JFileCh
From:       Philip Race <philip.race () oracle ! com>
Date:       2016-10-25 15:01:13
Message-ID: 580F73B9.60509 () oracle ! com
[Download RAW message or body]

I am not respinning again. The backout is all I am willing to take for 
this PIT.
It is the lowest risk option and SQE already says it fixes the problem.
If someone wants to file yet-another-bug for this updated version
and take on the task of running all related regression tests it can be done
in the regular repository after PIT is complete.

-phil

On 10/25/16, 3:10 AM, Semyon Sadetsky wrote:
>
> Looks good.
>
> --Semyon
>
>
> On 10/25/2016 12:21 PM, Alexandr Scherbatiy wrote:
>>
>> Could you review the updated fix:
>> http://cr.openjdk.java.net/~alexsch/8167988/webrev.00
>>
>>   - Path creation is moved under instanceof ShellFolder check
>>   - Paths.get(file.getPath()) is changed to file.toPath()
>>
>>   Thanks,
>>   Alexandr.
>>
>> On 10/25/2016 9:36 AM, Semyon Sadetsky wrote:
>>>
>>> It seems the fix should be:
>>>
>>> ===================================================================
>>> --- src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
>>> +++ src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
>>> @@ -244,10 +244,10 @@
>>>        * @exception FileNotFoundException if file does not exist
>>>        */
>>>       public static ShellFolder getShellFolder(File file) throws FileNotFoundException {
>>> -        Path path = Paths.get(file.getPath());
>>>           if (file instanceof ShellFolder) {
>>>               return (ShellFolder)file;
>>>           }
>>> +        Path path = Paths.get(file.getPath());
>>>           if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) {
>>>               throw new FileNotFoundException();
>>>           }
>>> ===================================================================
>>>
>>> --Semyon
>>> On 10/25/2016 1:26 AM, Phil Race wrote:
>>>> SQE reports some 35 tests fail as a result of this bug.
>>>> I am backing out the change that caused it from our PIT and need a 
>>>> reviewer.
>>>> SQE will verify this before I push.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8167988
>>>>
>>>> Fix :
>>>>
>>>> hg diff src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
>>>> diff --git 
>>>> a/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java 
>>>> b/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
>>>> --- a/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
>>>> +++ b/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
>>>> @@ -30,10 +30,6 @@
>>>>  import java.awt.Toolkit;
>>>>  import java.io.*;
>>>>  import java.io.FileNotFoundException;
>>>> -import java.nio.file.Files;
>>>> -import java.nio.file.LinkOption;
>>>> -import java.nio.file.Path;
>>>> -import java.nio.file.Paths;
>>>>  import java.util.*;
>>>>  import java.util.concurrent.Callable;
>>>>
>>>> @@ -244,11 +240,10 @@
>>>>       * @exception FileNotFoundException if file does not exist
>>>>       */
>>>>      public static ShellFolder getShellFolder(File file) throws 
>>>> FileNotFoundException {
>>>> -        Path path = Paths.get(file.getPath());
>>>>          if (file instanceof ShellFolder) {
>>>>              return (ShellFolder)file;
>>>>          }
>>>> -        if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) {
>>>> +        if (!file.exists()) {
>>>>              throw new FileNotFoundException();
>>>>          }
>>>>          return shellFolderManager.createShellFolder(file);
>>>>
>>>
>>
>

[Attachment #3 (text/html)]

<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    I am not respinning again. The backout is all I am willing to take
    for this PIT.<br>
    It is the lowest risk option and SQE already says it fixes the
    problem.<br>
    If someone wants to file yet-another-bug for this updated version<br>
    and take on the task of running all related regression tests it can
    be done<br>
    in the regular repository after PIT is complete.<br>
    <br>
    -phil<br>
    <br>
    On 10/25/16, 3:10 AM, Semyon Sadetsky wrote:
    <blockquote
      cite="mid:5dd16aba-e176-1067-89c7-b8145249b6cd@oracle.com"
      type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      <p>Looks good.</p>
      <p>--Semyon<br>
      </p>
      <br>
      <div class="moz-cite-prefix">On 10/25/2016 12:21 PM, Alexandr
        Scherbatiy wrote:<br>
      </div>
      <blockquote
        cite="mid:5e58bcf4-21b4-4afd-ab81-2e43a80a0cf0@oracle.com"
        type="cite">
        <meta content="text/html; charset=UTF-8"
          http-equiv="Content-Type">
        <br>
        Could you review the updated fix:<br>
           <a moz-do-not-send="true" class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Ealexsch/8167988/webrev.00">http://cr.openjdk.java.net/~alexsch/8167988/webrev.00</a><br>
  <br>
           - Path creation is moved under instanceof ShellFolder check<br>
           - Paths.get(file.getPath()) is changed to file.toPath()<br>
        <br>
           Thanks,<br>
           Alexandr.<br>
        <br>
        <div class="moz-cite-prefix">On 10/25/2016 9:36 AM, Semyon
          Sadetsky wrote:<br>
        </div>
        <blockquote
          cite="mid:a5456b83-b628-aff8-8908-f9b8297e8509@oracle.com"
          type="cite">
          <meta content="text/html; charset=UTF-8"
            http-equiv="Content-Type">
          <p>It seems the fix should be:</p>
          <p>
            <meta http-equiv="content-type" content="text/html;
              charset=UTF-8">
          </p>
          <pre style="background-color:#ffffff;color:#000000;font-family:'Consolas';fo \
                nt-size:10.5pt;">===================================================================
                
--- src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
+++ src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
@@ -244,10 +244,10 @@
      * @exception FileNotFoundException if file does not exist
      */
     public static ShellFolder getShellFolder(File file) throws FileNotFoundException \
                {
-        Path path = Paths.get(file.getPath());
         if (file instanceof ShellFolder) {
             return (ShellFolder)file;
         }
+        Path path = Paths.get(file.getPath());
         if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) {
             throw new FileNotFoundException();
         }
===================================================================

--Semyon
</pre>
          <div class="moz-cite-prefix">On 10/25/2016 1:26 AM, Phil Race
            wrote:<br>
          </div>
          <blockquote
            cite="mid:cb82dc2d-b83c-77c4-48d4-a314ecfc11ab@oracle.com"
            type="cite">SQE reports some 35 tests fail as a result of
            this bug. <br>
            I am backing out the change that caused it from our PIT and
            need a reviewer. <br>
            SQE will verify this before I push. <br>
            <br>
            Bug: <a moz-do-not-send="true"
              class="moz-txt-link-freetext"
              href="https://bugs.openjdk.java.net/browse/JDK-8167988">https://bugs.openjdk.java.net/browse/JDK-8167988</a>
  <br>
            <br>
            Fix : <br>
            <br>
            hg diff
            src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
            <br>
            diff --git
            a/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
            b/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
            <br>
            ---
            a/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
            <br>
            +++
            b/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
            <br>
            @@ -30,10 +30,6 @@ <br>
              import java.awt.Toolkit; <br>
              import java.io.*; <br>
              import java.io.FileNotFoundException; <br>
            -import java.nio.file.Files; <br>
            -import java.nio.file.LinkOption; <br>
            -import java.nio.file.Path; <br>
            -import java.nio.file.Paths; <br>
              import java.util.*; <br>
              import java.util.concurrent.Callable; <br>
            <br>
            @@ -244,11 +240,10 @@ <br>
                       * @exception FileNotFoundException if file does not
            exist <br>
                       */ <br>
                     public static ShellFolder getShellFolder(File file)
            throws FileNotFoundException { <br>
            -               Path path = Paths.get(file.getPath()); <br>
                             if (file instanceof ShellFolder) { <br>
                                     return (ShellFolder)file; <br>
                             } <br>
            -               if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS))
            { <br>
            +               if (!file.exists()) { <br>
                                     throw new FileNotFoundException(); <br>
                             } <br>
                             return shellFolderManager.createShellFolder(file);
            <br>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic