<AWT Dev> Fwd: [8] Review request for 6550588: java.awt.Desktop cannot open file with Windows UNC filename

Artem Ananiev artem.ananiev at oracle.com
Mon Mar 18 10:56:12 PDT 2013


Looks fine.

Thanks,

Artem

On 3/14/2013 3:33 AM, Anton Litvinov wrote:
> Hello Artem,
>
> Could you please review a new version of the fix. In this version of the
> fix only the regression test was corrected by changing values of
> timeouts from 1000 ms to 5000 ms, as you earlier recommended, in order
> to provide the application used for opening of TXT files with more time
> for loading.
>
> Webrev: http://cr.openjdk.java.net/~alitvinov/6550588/webrev.02
>
> This solution was chosen instead of a theoretical solution involving the
> shell script, because of the following reasons:
> 1. Even the shell script cannot provide handy direct means for starting
> a process and getting its PID. The defined most trustworthy solution is
> starting "java.exe" in background with immediate getting of the last
> started PID.
>
>      ${TESTJAVA}/bin/java OpenByUNCPathNameTest &
>      javaPid=$!
>
> 2. Running Java regression test from a shell script, brought unsolvable
> problems with access privileges of JVM executing Java regression test.
> For example, "ShellExecute" function called from
> "java.awt.Desktop.open(file)" always fails with "Access Denied" error.
>
> Thank you,
> Anton
>
> On 3/12/2013 7:38 PM, Artem Ananiev wrote:
>>
>> On 3/11/2013 7:12 PM, Anton Litvinov wrote:
>>> Hello Artem,
>>>
>>> Thank you very much for review of this fix. A reason of pressing Alt+F4
>>> keys is a need to end a process, which was started as a result of a call
>>> to "java.awt.Desktop.open()" method. I think that the only other way to
>>> end the started process can be termination of all found child processes
>>> of JVM started by this regression test. And this can theoretically be
>>> implemented in an additional shell script associated with this
>>> regression test. Would such a possible solution be acceptable?
>>>
>>> The class "java.awt.Robot" was used, because I hoped that TXT extension
>>> would be associated with a windowed application like "notepad.exe" with
>>> 100% probability in a test environment, but now I realize that this can
>>> be not a case, and if no windowed application was opened the test will
>>> end just the wrong application.
>>
>> Yes, this is exactly the scenario that bothers me a lot. Could you try
>> to create a shell test, please? If it is more predictable, I would
>> prefer it over Robot approach.
>>
>> Thanks,
>>
>> Artem
>>
>>> Thank you,
>>> Anton
>>>
>>> On 3/11/2013 5:18 PM, Artem Ananiev wrote:
>>>> Hi, Anton,
>>>>
>>>> the fix looks fine. I've got a question about the test, though.
>>>>
>>>> Pressing Alt+F4 with robot is potentially harmful and can close an
>>>> arbitrary window instead a window opened by Desktop.open(). Is there
>>>> another way to close the window? If not, you should at least increase
>>>> the timeout from 1000ms to 5000ms or so.
>>>>
>>>> Thanks,
>>>>
>>>> Artem
>>>>
>>>> On 3/6/2013 5:44 PM, Anton Litvinov wrote:
>>>>> Hello Alexey,
>>>>>
>>>>> Could you please review the second version of the fix. This version of
>>>>> the fix is based on transferring of a file path to "ShellExecute"
>>>>> native
>>>>> function without its prior conversion to URI object. The fix
>>>>> changes one
>>>>> parameter name of the private method "ShellExecute" of
>>>>> "sun.awt.windows.WDesktopPeer" to make it reflect the fact that the
>>>>> method accepts file paths.
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~alitvinov/6550588/webrev.01
>>>>>
>>>>> I also would like to inform that all available 9 regression tests
>>>>> connected with "java.awt.Desktop" were run and no negative changes
>>>>> were
>>>>> observed.
>>>>>
>>>>> Thank you,
>>>>> Anton
>>>>>
>>>>> On 3/4/2013 5:29 PM, Alexey Utkin wrote:
>>>>>> The [ShellExecute] function signature is
>>>>>> HINSTANCE ShellExecute(
>>>>>>    _In_opt_  HWND hwnd,
>>>>>>    _In_opt_  LPCTSTR lpOperation,
>>>>>>    _In_      LPCTSTR lpFile,
>>>>>>    _In_opt_  LPCTSTR lpParameters,
>>>>>>    _In_opt_  LPCTSTR lpDirectory,
>>>>>>    _In_      INT nShowCmd
>>>>>> );
>>>>>>
>>>>>> http://msdn.microsoft.com/en-gb/library/windows/desktop/bb762153%28v=vs.85%29.aspx
>>>>>>
>>>>>>
>>>>>> "lpFile [in]
>>>>>>
>>>>>>     Type: LPCTSTR
>>>>>>     A pointer to a null-terminated string that specifies the file or
>>>>>>     object on
>>>>>>     which to execute the specified verb. To specify a Shell namespace
>>>>>>     object,
>>>>>>     pass the fully qualified parse name. Note that not all verbs are
>>>>>>     supported on all objects.
>>>>>>     For example, not all document types support the "print" verb.
>>>>>>     If a relative path is used for the lpDirectory parameter do not
>>>>>>     use a relative path for lpFile."
>>>>>>
>>>>>>
>>>>>> There is no a word about URI here. Could you try the suggested
>>>>>> approach with escaped paths?
>>>>>> In case of URI we are switching from the "file" entity to "object"
>>>>>> entity. But initially we got a file!
>>>>>>
>>>>>> If you prefer to use URI as file-object identifier, please use MS
>>>>>> spec
>>>>>> for URI:
>>>>>> http://msdn.microsoft.com/en-us/library/aa767731%28v=vs.85%29.aspx
>>>>>> +
>>>>>> https://blogs.oracle.com/alanb/entry/file_uris
>>>>>>
>>>>>> =================
>>>>>> The idea:
>>>>>> file:/ (Java) -> file:/// (MS) for any case of full path.
>>>>>> =================
>>>>>>
>>>>>> file:/c:/file.txt -> file:///c:/file.txt
>>>>>> file://localhost/file.txt -> file://///localhost/file.txt
>>>>>>
>>>>>> Regards,
>>>>>> -uta
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 04.03.2013 16:48, Anton Litvinov wrote:
>>>>>>> Hello Alexey,
>>>>>>>
>>>>>>> Thank you for a review of the fix and source code of a solution that
>>>>>>> you provided. Unfortunately, I do not think that the solution which
>>>>>>> encloses a file path into the quotation marks would be better,
>>>>>>> because Windows Shell function "ShellExecute" does not require
>>>>>>> presence of the quotation marks in a value of "LPCTSTR lpFile"
>>>>>>> parameter. Practically the function successfully handles both
>>>>>>> absolute file paths not enclosed into the quotation marks and
>>>>>>> enclosed. For example, the following two calls are executed
>>>>>>> successfully in my local environment:
>>>>>>>
>>>>>>> ::ShellExecute(NULL, _T("open"), _T("D:/Documents/Test Dir 1/Read
>>>>>>> Me.txt"), NULL, NULL, SW_SHOWNORMAL);
>>>>>>> ::ShellExecute(NULL, _T("open"), _T("\"D:/Documents/Test Dir 1/Read
>>>>>>> Me.txt\""), NULL, NULL, SW_SHOWNORMAL);
>>>>>>>
>>>>>>> The main reason of the bug is a way in which "java.io.File.toURI"
>>>>>>> method converts absolute file path to URI with the protocol "file"
>>>>>>> and the way in which "ShellExecute" function interprets this URI. In
>>>>>>> such a case it would make sense to remove all the code converting
>>>>>>> "java.io.File" pathnames to URI from the file
>>>>>>> "sun.awt.windows.WDesktopPeer.java" and to transfer the result of a
>>>>>>> call to "File.getAbsolutePath()" directly to the method
>>>>>>>
>>>>>>> private static native String ShellExecute(String uri, String verb);
>>>>>>>
>>>>>>> But the signature of the method states that it expects the URI, thus
>>>>>>> a transfer of Windows UNC pathname or a local Windows file path
>>>>>>> instead of a URI with "file" scheme will be incorrect. Also URI
>>>>>>> prohibits presence of the quotation marks (double-quote characters)
>>>>>>> in its body according to chapter "2.4.3. Excluded US-ASCII
>>>>>>> Characters" of "RFC 2396" available at
>>>>>>> (http://www.ietf.org/rfc/rfc2396.txt).
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Anton
>>>>>>>
>>>>>>> On 3/4/2013 2:16 PM, Alexey Utkin wrote:
>>>>>>>> It seems that file name escaping by ["] is better solution.
>>>>>>>> http://www.speechcomputing.com/node/2577
>>>>>>>>
>>>>>>>>>     private static boolean isQuoted(String arg, String
>>>>>>>>> errorMessage) {
>>>>>>>>>         int lastPos = arg.length() - 1;
>>>>>>>>>         if (lastPos >=1 && arg.charAt(0) == '"' &&
>>>>>>>>> arg.charAt(lastPos) == '"') {
>>>>>>>>>             // The argument has already been quoted.
>>>>>>>>>             if (arg.indexOf('"', 1) != lastPos) {
>>>>>>>>>                 // There is ["] inside.
>>>>>>>>>                 throw new IllegalArgumentException(errorMessage);
>>>>>>>>>             }
>>>>>>>>>             return true;
>>>>>>>>>         }
>>>>>>>>>         if (arg.indexOf('"') >= 0) {
>>>>>>>>>             // There is ["] inside.
>>>>>>>>>             throw new IllegalArgumentException(errorMessage);
>>>>>>>>>         }
>>>>>>>>>         return false;
>>>>>>>>>     }
>>>>>>>>>
>>>>>>>>>     private static String getExecutablePath(File file)
>>>>>>>>>         throws IOException
>>>>>>>>>     {
>>>>>>>>>         String path = file.getPath();
>>>>>>>>>         boolean pathIsQuoted = isQuoted(path,
>>>>>>>>>                 "File name has embedded quote");
>>>>>>>>>         return pathIsQuoted
>>>>>>>>>             ? path
>>>>>>>>>             : ("\"" + path + "\"");
>>>>>>>>>     }
>>>>>>>>
>>>>>>>>  this.ShellExecute(getExecutablePath(file), ACTION_XXXX_VERB);
>>>>>>>>
>>>>>>>> That reduces the injection scenario and is more compatible with
>>>>>>>> [ShellExecute] spec:
>>>>>>>> http://msdn.microsoft.com/en-gb/library/windows/desktop/bb762153%28v=vs.85%29.aspx
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> -uta
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 01.03.2013 19:17, Artem Ananiev wrote:
>>>>>>>>>
>>>>>>>>> Your comments are welcome ;)
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Artem
>>>>>>>>>
>>>>>>>>> -------- Original Message --------
>>>>>>>>> Subject:     <AWT Dev> [8] Review request for 6550588:
>>>>>>>>> java.awt.Desktop
>>>>>>>>> cannot open file with Windows UNC filename
>>>>>>>>> Date:     Fri, 01 Mar 2013 18:38:03 +0400
>>>>>>>>> From:     Anton Litvinov <anton.litvinov at oracle.com>
>>>>>>>>> Organization:     Oracle Corporation
>>>>>>>>> To: awt-dev at openjdk.java.net
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Please review the following fix for a bug.
>>>>>>>>>
>>>>>>>>> Bug: http://bugs.sun.com/view_bug.do?bug_id=6550588
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~alitvinov/6550588/webrev.00
>>>>>>>>>
>>>>>>>>> The bug consists in inability to open a file with Windows UNC
>>>>>>>>> pathname
>>>>>>>>> by means of "java.awt.Desktop.open" method. The solution adds
>>>>>>>>> code to
>>>>>>>>> "sun.awt.windows.WDesktopPeer" class which modifies URI received
>>>>>>>>> as a
>>>>>>>>> result of a call to "java.io.File.toURI" method to make it
>>>>>>>>> satisfy the
>>>>>>>>> requirements of Windows API concerning a number of consecutive '/'
>>>>>>>>> characters following a scheme part of URI. Also regression tests
>>>>>>>>> related
>>>>>>>>> to "java.awt.Desktop" were run on Windows XP and Windows 7, no
>>>>>>>>> negative
>>>>>>>>> changes were detected.
>>>>>>>>>
>>>>>>>>> A comment with the latest information about the analysis of this
>>>>>>>>> issue
>>>>>>>>> was added to the bug's page, but it is not available at
>>>>>>>>> "http://bugs.sun.com" yet, because of the time required for
>>>>>>>>> synchronization. Therefore it is provided below.
>>>>>>>>>
>>>>>>>>> The comment:
>>>>>>>>>
>>>>>>>>>     During analysis of this bug the following facts were defined:
>>>>>>>>>     1. URI strings constructed from Windows UNC pathnames like
>>>>>>>>> former
>>>>>>>>>     mentioned "\\host\path\to\f i l e.txt" can still be handled by
>>>>>>>>>     "ShellExecute()" Windows Shell function, if the URI string is
>>>>>>>>> not
>>>>>>>>>     encoded. Presence of space characters in the URI string
>>>>>>>>> does not
>>>>>>>>>     make the function fail, for example
>>>>>>>>> "file:////host/path/to/f i l
>>>>>>>>>     e.txt" can be successfully processed by "ShellExecute()"
>>>>>>>>> function.
>>>>>>>>>     2. Windows API is designed to handle URI strings with "file"
>>>>>>>>>     protocol scheme correctly, when the strings have certain
>>>>>>>>> number of
>>>>>>>>>     '/' characters after the scheme name:
>>>>>>>>>          - 2 slashes for URI converted from a Windows UNC
>>>>>>>>> pathname.
>>>>>>>>> For
>>>>>>>>>     example, "\\host\path\to\f i l e.txt" corresponds to the URI
>>>>>>>>> "file://host/path/to/f%20i%20l%20e.txt".
>>>>>>>>>          - 3 slashes for URI converted from a local Windows file
>>>>>>>>> path.
>>>>>>>>>     For example, "C:\Temp Dir\f i l e.txt" corresponds to the URI
>>>>>>>>> "file:///C:/Temp%20Dir/f%20i%20l%20e.txt".
>>>>>>>>>          This fact is described in the article at the following
>>>>>>>>> URL
>>>>>>>>>
>>>>>>>>> (http://blogs.msdn.com/b/ie/archive/2006/12/06/file-uris-in-windows.aspx).
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>     3. Current implementation of the class "java.io.File" converts
>>>>>>>>>     abstract file names to URI in the following way:
>>>>>>>>>          - "C:\Temp\File.txt" -> "file:/C:/Temp/File.txt".
>>>>>>>>>          - "\\host\SharedFolder\Temp\File.txt" ->
>>>>>>>>> "file:////host/SharedFolder/Temp/File.txt".
>>>>>>>>>
>>>>>>>>>     Since "java.io.File" is cross-platform and stable, perhaps,
>>>>>>>>>     additional modification of the URI string to the format
>>>>>>>>> expected by
>>>>>>>>>     Windows API can be implemented in Windows specific part of
>>>>>>>>>     "java.awt.Desktop" class.
>>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Anton
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>



More information about the awt-dev mailing list