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 <[email protected]>
Organization: Oracle Corporation
To: [email protected]
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
