View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0021659 | Lazarus | LCL | public | 2012-04-06 14:05 | 2014-10-24 15:58 |
| Reporter | BigChimp | Assigned To | Bart Broersma | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | x64 | OS | Windows | OS Version | Vista |
| Product Version | 0.9.31 (SVN) | Product Build | 36600 | ||
| Target Version | Fixed in Version | 1.1 (SVN) | |||
| Summary | 0021659: [Patch] OpenURL on Windows needs double quotes for file:// URL | ||||
| Description | Calling OpenURL on Windows with a path+file with spaces in it does not work; calling it enclosed in " does work. Documentation specifies "AURL must be an URL like "... "file://C:\test.txt."... and doesn't mention double quotes. Confirmed by Felipe (see below). | ||||
| Steps To Reproduce | Use test program from 21651. Select a pdf with spaces in the path. Click OpenURL PDF does not open... but neither is an error message generated. Apply patch; pdf should open now. | ||||
| Additional Information | Tested on Win Vista x64; not tested on WinCE or Win 9x (ShellExecuteA) range. See Lazarus mailing list dicussion todo, subject lclintf.OpenURL parameter: double quotes or not? On 6-4-2012 13:30, Felipe Monteiro de Carvalho wrote: > You should create a bug report (or a patch). The Windows code of > OpenURL should detect the spaces and add the quotes. Patch included which strictly checks for file:// URLs. No idea if or what checks should be done on http:// etc URLs... | ||||
| Tags | openurl | ||||
| Fixed in Revision | 36744, 36745, 36934, 40490 | ||||
| LazTarget | - | ||||
| Widgetset | Win32/Win64 | ||||
| Attached Files |
| ||||
| related to | 0021651 | closed | Felipe Monteiro de Carvalho | [Patch] OpenDocument requires " around file on OSX, not on Windows |
| related to | 0026091 | resolved | Bart Broersma | LCLIntf openurl() / LazHelpHTML - HtmlBrowserHelpViewer can not pass query string or anchor target |
|
2012-04-06 14:05
|
OpenURLQuotes.diff (1,655 bytes)
Index: lcl/include/sysenvapis_win.inc
===================================================================
--- lcl/include/sysenvapis_win.inc (revision 36600)
+++ lcl/include/sysenvapis_win.inc (working copy)
@@ -2,6 +2,8 @@
// Open a given URL with the default browser
function OpenURL(AURL: String): Boolean;
+const
+ FileURL='file://';
var
{$IFDEF WinCE}
Info: SHELLEXECUTEINFO;
@@ -9,26 +11,38 @@
ws: WideString;
ans: AnsiString;
{$ENDIF}
+ ResultingURL: string;
begin
Result := False;
if AURL = '' then Exit;
+ if Copy(AURL, 1, Length(FileURL))=FileURL then
+ begin
+ // Enclose file URL with double quotes; needed if there are spaces etc. in URL
+ ResultingURL:='"'+AURL+'"';
+ end
+ else
+ begin
+ // Either it's not a file URL or it has already been quoted
+ ResultingURL:=AURL;
+ end;
+
{$IFDEF WinCE}
FillChar(Info, SizeOf(Info), 0);
Info.cbSize := SizeOf(Info);
Info.fMask := SEE_MASK_FLAG_NO_UI;
Info.lpVerb := 'open';
- Info.lpFile := PWideChar(UTF8Decode(AURL));
+ Info.lpFile := PWideChar(UTF8Decode(ResultingURL));
Result := ShellExecuteEx(@Info);
{$ELSE}
if Win32Platform = VER_PLATFORM_WIN32_NT then
begin
- ws := UTF8Decode(AURL);
+ ws := UTF8Decode(ResultingURL);
Result := ShellExecuteW(0, 'open', PWideChar(ws), nil, nil, SW_SHOWNORMAL) > 32;
end
else
begin
- ans := Utf8ToAnsi(AURL); // utf8 must be converted to Windows Ansi-codepage
+ ans := Utf8ToAnsi(ResultingURL); // utf8 must be converted to Windows Ansi-codepage
Result := ShellExecute(0, 'open', PAnsiChar(ans), nil, nil, SW_SHOWNORMAL) > 32;
end;
{$ENDIF}
|
|
|
I wonder if it should only add quotes when there are actually spaces. It would be safer. |
|
|
Why do you think that? The patch works fine in paths without spaces. Note: I couldn't find any documentation on using URLs with ShellExecute... do you know of any? |
|
|
Added test program to 0021651... sorry, had forgotten that |
|
|
Here is an explanation of Opening URLS with ShellExecute: http://support.microsoft.com/kb/224816 HKEY_CLASSES_ROOT\http\shell\open\command on my system hast the value "C:\Program Files\Mozilla Firefox\firefox.exe" -requestPending -osint -url "%1". Since firefox doesn't require url's to be encoded, it works without encoding the url. Possibly this is browser dependent. http://msdn.microsoft.com/en-us/library/windows/desktop/bb776776%28v=vs.85%29.aspx#sec_shell_shellexec says that arguments with spaces have to be quoted. |
|
|
Maybe, if we use the file:// URI scheme, we need to use HTML escape codes for spaces in the URL? For instance file://C:/some%20file.pdf ShellExecute itself does not need quotes around the filespecs, only around additional args if specified AFAIR As pointed out in mailinglist OpenDocument('some file.pfd') succeeds, whereas OpenDocument('file://c:/some file.pdf') fails. (Mantis breaks this URL) |
|
|
There is similar problem with #. With this example OpenDocument doesn't jump to anchor: file://E:\SomeFile.html#JumpToHere It just opens file://E:\SomeFile.html. If quoted, it does jump. Escaping doesn't work. For example: file://E:\SomeFile.html%23JumpToHere doesn't open anything at all. If I enter file://E:\SomeFile.html#JumpToHere in MenuStart/Run it also does not jump. If I enter it quoted it does jump. I use Mozilla Firefox. |
|
|
HtmlEscaping indeed does not work. @cobines: escaping a '#' in an url means that the '#' is actually part of the filename. On my system (win7 64 bit, 32-bit Lazarus) the OpenUrl works fine with or without quoting when using a filename with spaces in it. Jumping to a local anchor does not seem to work weather quoted or not. Mind you I also cannot jump to local anchor if I supply the "file://somefile#localanchor" in the windows start menu (Start -> Run -> ...) My default browser is FF (same shell command as Ludo). |
|
|
This seems to depend on the process that interprets the command line. Entered in Run->Start: Opens "some file.html", doesn't jump to anchor: file://E:\some file.html#jump "file://E:\some file.html#jump" C:\WINDOWS\system32\rundll32.exe url.dll,FileProtocolHandler file://E:\some file.html#jump C:\WINDOWS\system32\rundll32.exe url.dll,FileProtocolHandler file:///E:\some%20file.html#jump Tries to open "file://E:\some" and "file.html#jump" "c:\Program Files\Mozilla Firefox\firefox.exe" file://E:\some file.html#jump "C:\Program Files\Opera\Opera.exe" file://E:\some file.html#jump These work OK: "c:\Program Files\Mozilla Firefox\firefox.exe" file://E:\some%20file.html#jump "c:\Program Files\Mozilla Firefox\firefox.exe" "file://E:\some file.html#jump" "C:\Program Files\Opera\Opera.exe" file://E:\some%20file.html#jump "C:\Program Files\Opera\Opera.exe" "file://E:\some file.html#jump" C:\WINDOWS\system32\rundll32.exe url.dll,FileProtocolHandler "file://E:\some file.html#jump" The url.dll,FileProtocolHandler function seems to need quoting (on Windows XP). Maybe on later versions they changed it though. |
|
|
Looking at Ludo's and cobines reactions, it would seem to make sense to simply always double quote as browsers/applications must be able to handle this (it's the only way to pass a path with spaces). IIUC, always quoting, regardless of spaces in path, will also enable jumps to anchors (#) within URLs. @Bart Broersma 2012-04-07 14:58: "ShellExecute itself does not need quotes around the filespecs, only around additional args if specified AFAIR": as Ludo's link indicated, shellexecute needs double quotes when passing URLs with spaces in them... Further behaviour seems application dependent. Suggest we wait for problems with other browsers - if any - to see if further patches are required. Summary: suggest implementing this or a similar patch... |
|
|
@reinier > as Ludo's link indicated, shellexecute needs double quotes when passing URLs with > spaces in them... This is not true. The code fragment in the link from Ludo's post indeed uses double quotes, but only because this code example is in C (and the url is supplied as a string constant), and C uses doube-quotes instead of the Pascal single-quotes. |
|
|
So far tested with FF, Opera and IE as default browser. Results are all the same: Using file:// URI scheme, filename with spaces, not quoted: OK Using file:// URI scheme, filename with spaces, quoted: OK So, in these cases quoting makes no difference. No browser will jump to a local anchor, wether quoted or not. Tested on Win7 64-bit. I'll test on Win9x also. |
|
2012-04-10 01:00
|
test file.html (457 bytes) |
|
|
I've uploaded the test file I used for reference. |
|
|
@Bart: thanks. Do you think we should use single quotes then? @all: if not already doing so, suggest you test with a Lazarus program such as (an adaptation of) 0021651 to make results more repeatable and valid... |
|
|
> @Bart: thanks. Do you think we should use single quotes then? I think windows typically expects double quotes around parametrs, if and when quotes are expected. Never tested this though. |
|
|
> file:// URI scheme, filename with spaces, not quoted: OK This doesn't mean anything. Read the first link I provided on how ShellExecute works. Then look with regedit and find the Shell/Open entry for file: there isn't any. There is a 'Source Filter' that points to quartz.dll which is directshow. Nothing to do with Shell Open. So there is no default Open for file: schemes and the shell will use the file extension. If your file is .doc it will open word or wordpad or whatever is registered for .doc. The application opened decides wheter spaces have to be encoded while the shell determines if names with spaces should be quoted in order to pass it as one parameter. > The code fragment in the link from Ludo's post indeed uses double quotes, but only because this code example is in C The link I provided says "If you provide a command-line string that contains white space, wrap the string in quotation marks. Otherwise, the parser might interpret a single element that contains spaces as multiple elements." It doesn't say double qoutes. The quotes have nothing to do with the programming language but with the shell. |
|
|
Tested on WinMe with Opera and FireFox: If you wrap the file:// URI scheme in quotation marks OpenUrl() fails. @Ludo >> file:// URI scheme, filename with spaces, not quoted: OK > This doesn't mean anything. The topic of this bugreport is wether OpenUrl with file:// URI scheme _needs_ quotes (on Windows). I merely reported the testing results on my system. > The link I provided says "If you provide a command-line string that contains > white space, wrap the string in quotation marks. I looked at wrong link, sorry. I have never seen that this was necessary for the lpFile paramater, but only for lpParameters. E.g. ShellExecute(0, 'open', 'path\to\someapp.exe', 'some text file.txt', nil, SW_SHOWNORMAL) --> Fail ShellExecute(0, 'open', 'some text file', nil, nil, SW_SHOWNORMAL) --> OK This may however be the result of how apps generally regeister themselfs: My default http:// command in the registry: E:\PROGRA~1\MOZILL~1.3\FIREFOX.EXE -requestPending -osint -url "%1" This probably means that ShellExecute(0,'open','some file.htm',...) will result in ...\firefox.exe -requestPending -osint -url "some file.htm" on my system. Conclusions so far: Quoting a file:// URL works on some systems, but fails on others. Quoting a file:// URL does not guarantee jumping to local anchors in html-files. On windows it does not seem necessary to supply the URL to a local file in the file:// URI scheme format. (If our documentatiosn says otherwise, maybe we should adjust our documentation) Currently I don't see how this can be fixed. I think it's the responsability of the programmer to supply a valid URL to for the OpenUrl() function. |
|
|
@Bart: just to make sure: are you confirming the test program fails with the patch on WinME (with Firefox/Opera)? "I think it's the responsability of the programmer to supply a valid URL to for the OpenUrl() function."=> if it is not clear what a valid URL is... deferring this decision to the end user does not help. |
|
|
@Reinier: On WinMe with unpatched OpenUrl: OpenUrl('file://path/to/file_with_spaces.htm') -> OK OpenUrl('"file://path/to/file_with_(or_without)_spaces.htm"') -> Fail (the underscores are there to fool Mantis, in actual testing they are spaces) and for the record: OpenUrl('path/to/file with spaces.htm') -> OK OpenUrl('"path/to/file with spaces.htm"') -> OK (So the patch would fail on WinMe) On Windows what would be the advantage of the using file:// URI scheme over using plain filename (quoted or otherwise) as parameter to OpenUrl()? So another approach could be (on Windows) to remove file:// from the URL if it is specified in the AURL parameter. |
|
|
The url should start with file:/// (three slashes). See for example http://en.wikipedia.org/wiki/File_URI_scheme. Though I doubt this will change anything. |
|
|
I think that at this point it is clear that the patch is wrong. OpenURL opens a URL. If you are writing a URL you should know that it needs to be encoded according to these rules: http://www.ietf.org/rfc/rfc1738.txt <quote> Characters can be unsafe for a number of reasons. The space character is unsafe because significant spaces may disappear and insignificant spaces may be introduced when URLs are transcribed or typeset or subjected to the treatment of word-processing programs. The characters "<" and ">" are unsafe because they are used as the delimiters around URLs in free text; the quote mark (""") is used to delimit URLs in some systems. The character "#" is unsafe and should always be encoded because it is used in World Wide Web and in other systems to delimit a URL from a fragment/anchor identifier that might follow it. The character "%" is unsafe because it is used for encodings of other characters. Other characters are unsafe because gateways and other transport agents are known to sometimes modify such characters. These characters are "{", "}", "|", "\", "^", "~", "[", "]", and "`". All unsafe characters must always be encoded within a URL. For example, the character "#" must be encoded within URLs even in systems that do not normally deal with fragment or anchor identifiers, so that if the URL is copied into another system that does use them, it will not be necessary to change the URL encoding. </quote> But anyway I will make our routine more resistent by adding a quick check which will properly encode all spaces which it receives in Windows, if the user passed any spaces. |
|
|
The patch in r36745 is wrong. It will render the OpenDocument() function useless for any file that has spaces in it (see note: 0058408). I think it is the programmers responsability to provide a proper URL to this function. Please revert the changes made in r36745 or alternatively provide a new implementation of OpenDocument(). |
|
|
@Felipe: well, thanks for the quick heads up. @Cobines, no: file://c:\blabla (or the encoded version) would work on Windows, fiel :///blabla would make sense on Unix/Linux |
|
|
Reinier probably closed this issue at almost the same time as I re-opened it and probably did not reed/see my note. Escaping spaces with %20 in URL's in the OpenURL function breaks OpenDocument (see note: 0058408). Also this patch introduces different behaviour between Win, Linux and Mac. Please revert. |
|
2012-04-18 23:00
|
opendoc.zip (2,925 bytes) |
|
|
Attached demo shows the broken OpenDocument from r36745 Using r36745: C:\Users\Bart\LazarusProjecten\bugs\OpenUrl>opendoc S = C:\Users\Bart\LazarusProjecten\bugs\OpenUrl\test text.txt --> Fail S = "C:\Users\Bart\LazarusProjecten\bugs\OpenUrl\test text.txt" --> Fail S = file://C:\Users\Bart\LazarusProjecten\bugs\OpenUrl\test text.txt --> Fail S = "file://C:\Users\Bart\LazarusProjecten\bugs\OpenUrl\test text.txt" --> Fail After reverting r36745: C:\Users\Bart\LazarusProjecten\bugs\OpenUrl>opendoc S = C:\Users\Bart\LazarusProjecten\bugs\OpenUrl\test text.txt --> OK S = "C:\Users\Bart\LazarusProjecten\bugs\OpenUrl\test text.txt" --> OK S = file://C:\Users\Bart\LazarusProjecten\bugs\OpenUrl\test text.txt --> OK S = "file://C:\Users\Bart\LazarusProjecten\bugs\OpenUrl\test text.txt" --> OK (Tested on Win7 (the last line always fails on Win9x)) |
|
|
Oh ... what a nest of snakes this issue is =) Reverted. |
|
|
Thanks all for responding. Felipe, noticed you set the issue to resolved, so I'm closing it. I still think a note in the documentation should mention that the format of URLs is platform dependent (possibly application-dependent?) in order to limit nasty surprises. If necessary, I might open another issue on OpenDocument... |
|
|
Quote file:// url's on NT platform if they contain spaces. Should fix original issue, and not break Win9x. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2012-04-06 14:05 |
|
New Issue | |
| 2012-04-06 14:05 |
|
File Added: OpenURLQuotes.diff | |
| 2012-04-06 14:05 |
|
Widgetset | => Win32/Win64 |
| 2012-04-06 15:03 | Felipe Monteiro de Carvalho | Note Added: 0058356 | |
| 2012-04-06 15:12 | Felipe Monteiro de Carvalho | Status | new => assigned |
| 2012-04-06 15:12 | Felipe Monteiro de Carvalho | Assigned To | => Felipe Monteiro de Carvalho |
| 2012-04-06 15:13 | Felipe Monteiro de Carvalho | Relationship added | related to 0021651 |
| 2012-04-06 15:24 |
|
Note Added: 0058359 | |
| 2012-04-07 09:07 |
|
Note Added: 0058383 | |
| 2012-04-07 09:08 |
|
Note Edited: 0058359 | |
| 2012-04-07 10:01 | Ludo Brands | Note Added: 0058385 | |
| 2012-04-07 14:58 | Bart Broersma | Note Added: 0058394 | |
| 2012-04-07 15:00 | Bart Broersma | Note Edited: 0058394 | |
| 2012-04-07 23:50 | cobines | Note Added: 0058401 | |
| 2012-04-08 18:29 | Bart Broersma | Note Added: 0058408 | |
| 2012-04-08 23:59 | Bart Broersma | Note Edited: 0058408 | |
| 2012-04-09 02:31 | cobines | Note Added: 0058420 | |
| 2012-04-09 02:34 | cobines | Note Edited: 0058420 | |
| 2012-04-09 02:35 | cobines | Note Edited: 0058420 | |
| 2012-04-09 02:35 | cobines | Note Edited: 0058420 | |
| 2012-04-09 13:22 |
|
Note Added: 0058428 | |
| 2012-04-09 13:24 |
|
Note Edited: 0058428 | |
| 2012-04-09 16:27 | Bart Broersma | Note Added: 0058437 | |
| 2012-04-09 23:42 | Bart Broersma | Note Added: 0058442 | |
| 2012-04-09 23:44 | Bart Broersma | Note Edited: 0058442 | |
| 2012-04-10 01:00 | cobines | File Added: test file.html | |
| 2012-04-10 01:06 | cobines | Note Added: 0058444 | |
| 2012-04-10 08:15 |
|
Note Added: 0058446 | |
| 2012-04-10 08:31 | Bart Broersma | Note Added: 0058447 | |
| 2012-04-10 08:59 | Ludo Brands | Note Added: 0058448 | |
| 2012-04-12 19:09 | Bart Broersma | Note Added: 0058538 | |
| 2012-04-12 19:26 | Bart Broersma | Note Edited: 0058538 | |
| 2012-04-12 19:46 |
|
Note Added: 0058539 | |
| 2012-04-12 20:25 | Bart Broersma | Note Added: 0058540 | |
| 2012-04-12 20:27 | Bart Broersma | Note Edited: 0058540 | |
| 2012-04-12 20:27 | Bart Broersma | Note Edited: 0058540 | |
| 2012-04-12 20:44 | Bart Broersma | Note Edited: 0058540 | |
| 2012-04-13 01:57 | cobines | Note Added: 0058546 | |
| 2012-04-13 14:46 | Felipe Monteiro de Carvalho | Note Added: 0058550 | |
| 2012-04-13 14:47 | Felipe Monteiro de Carvalho | Fixed in Revision | => 36744, 36745 |
| 2012-04-13 14:47 | Felipe Monteiro de Carvalho | LazTarget | => - |
| 2012-04-13 14:47 | Felipe Monteiro de Carvalho | Status | assigned => resolved |
| 2012-04-13 14:47 | Felipe Monteiro de Carvalho | Fixed in Version | => 1.1 (SVN) |
| 2012-04-13 14:47 | Felipe Monteiro de Carvalho | Resolution | open => fixed |
| 2012-04-13 15:07 | Bart Broersma | Status | resolved => assigned |
| 2012-04-13 15:07 | Bart Broersma | Resolution | fixed => reopened |
| 2012-04-13 15:07 | Bart Broersma | Note Added: 0058551 | |
| 2012-04-13 15:07 |
|
Status | assigned => closed |
| 2012-04-13 15:07 |
|
Note Added: 0058552 | |
| 2012-04-13 15:07 |
|
Resolution | reopened => no change required |
| 2012-04-13 15:43 | Bart Broersma | Status | closed => assigned |
| 2012-04-13 15:43 | Bart Broersma | Resolution | no change required => reopened |
| 2012-04-13 15:43 | Bart Broersma | Note Added: 0058553 | |
| 2012-04-17 10:01 | Bart Broersma | Note Edited: 0058553 | |
| 2012-04-18 23:00 | Bart Broersma | File Added: opendoc.zip | |
| 2012-04-18 23:07 | Bart Broersma | Note Added: 0058733 | |
| 2012-04-20 17:03 | Felipe Monteiro de Carvalho | Fixed in Revision | 36744, 36745 => 36744, 36745, 36934 |
| 2012-04-20 17:03 | Felipe Monteiro de Carvalho | Status | assigned => resolved |
| 2012-04-20 17:03 | Felipe Monteiro de Carvalho | Resolution | reopened => fixed |
| 2012-04-20 17:03 | Felipe Monteiro de Carvalho | Note Added: 0058804 | |
| 2012-04-20 17:56 |
|
Status | resolved => closed |
| 2012-04-20 17:56 |
|
Note Added: 0058808 | |
| 2013-03-06 00:08 | Bart Broersma | Assigned To | Felipe Monteiro de Carvalho => Bart Broersma |
| 2013-03-06 00:08 | Bart Broersma | Status | closed => assigned |
| 2013-03-06 00:08 | Bart Broersma | Resolution | fixed => reopened |
| 2013-03-06 00:10 | Bart Broersma | Fixed in Revision | 36744, 36745, 36934 => 36744, 36745, 36934, 40490 |
| 2013-03-06 00:10 | Bart Broersma | Note Added: 0066045 | |
| 2013-03-06 00:10 | Bart Broersma | Status | assigned => resolved |
| 2013-03-06 00:10 | Bart Broersma | Resolution | reopened => fixed |
| 2013-03-06 00:10 | Bart Broersma | Status | resolved => closed |
| 2014-10-24 15:58 |
|
Tag Attached: openurl | |
| 2014-10-24 15:58 |
|
Relationship added | related to 0026091 |
