<AWT Dev> Fwd: [8] Review request for 6550588: java.awt.Desktop cannot open file with Windows UNC filename
Anton Litvinov
anton.litvinov at oracle.com
Thu Mar 14 03:33:46 PDT 2013
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