Fix path/URL confusion in wxLaunchDefaultBrowser()

Add a helper wxLaunchBrowserParams struct with clearly distinct "url" and
"path" fields and GetPathOrURL() accessor which returns whichever is
appropriate.

This makes the code more clear and ensures that we never pass URLs (but only
file paths) to xdg-open under Unix as it doesn't handle them.

See #17227.

include/wx/private/launchbrowser.h

@@ -0,0 +1,43 @@
+///////////////////////////////////////////////////////////////////////////////
+// Name:        wx/private/launchbrowser.h
+// Purpose:     Helpers for wxLaunchDefaultBrowser() implementation.
+// Author:      Vadim Zeitlin
+// Created:     2016-02-07
+// Copyright:   (c) 2016 Vadim Zeitlin <vadim@wxwidgets.org>
+// Licence:     wxWindows licence
+///////////////////////////////////////////////////////////////////////////////
+
+#ifndef _WX_PRIVATE_LAUNCHBROWSER_H_
+#define _WX_PRIVATE_LAUNCHBROWSER_H_
+
+// ----------------------------------------------------------------------------
+// wxLaunchBrowserParams: passed to wxDoLaunchDefaultBrowser()
+// ----------------------------------------------------------------------------
+
+struct wxLaunchBrowserParams
+{
+    explicit wxLaunchBrowserParams(int f) : flags(f) { }
+
+    // Return either the URL or the file depending on our scheme.
+    const wxString& GetPathOrURL() const
+    {
+        return scheme == wxS("file") ? path : url;
+    }
+
+
+    // The URL is always specified and is the real URL, always with the scheme
+    // part, which can be "file://".
+    wxString url;
+
+    // The path is a local path which is only non-empty if the URL uses the
+    // "file://" scheme.
+    wxString path;
+
+    // The scheme of the URL, e.g. "file" or "http".
+    wxString scheme;
+
+    // The flags passed to wxLaunchDefaultBrowser().
+    int flags;
+};
+
+#endif // _WX_PRIVATE_LAUNCHBROWSER_H_

src/common/utilscmn.cpp

@@ -69,8 +69,10 @@
 #include <errno.h>
 
 #if wxUSE_GUI
+    #include "wx/filesys.h"
     #include "wx/notebook.h"
     #include "wx/statusbr.h"
+    #include "wx/private/launchbrowser.h"
 #endif // wxUSE_GUI
 
 #include <time.h>
@@ -84,7 +86,6 @@
 
 #if defined(__WINDOWS__)
     #include "wx/msw/private.h"
-    #include "wx/filesys.h"
 #endif
 
 #if wxUSE_GUI && defined(__WXGTK__)
@@ -1017,20 +1018,17 @@ bool wxSetDetectableAutoRepeat( bool WXUNUSED(flag) )
 // Launch default browser
 // ----------------------------------------------------------------------------
 
-#if defined(__WINDOWS__)
+#if defined(__WINDOWS__) || \
+    defined(__WXX11__) || defined(__WXGTK__) || defined(__WXMOTIF__) || \
+    defined(__WXOSX__)
 
 // implemented in a port-specific utils source file:
-bool wxDoLaunchDefaultBrowser(const wxString& url, const wxString& scheme, int flags);
+bool wxDoLaunchDefaultBrowser(const wxLaunchBrowserParams& params);
-
-#elif defined(__WXX11__) || defined(__WXGTK__) || defined(__WXMOTIF__) || defined(__WXOSX__)
-
-// implemented in a port-specific utils source file:
-bool wxDoLaunchDefaultBrowser(const wxString& url, int flags);
 
 #else
 
 // a "generic" implementation:
-bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
+bool wxDoLaunchDefaultBrowser(const wxLaunchBrowserParams& params)
 {
     // on other platforms try to use mime types or wxExecute...
 
@@ -1044,7 +1042,7 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
         wxString mt;
         ft->GetMimeType(&mt);
 
-        ok = ft->GetOpenCommand(&cmd, wxFileType::MessageParameters(url));
+        ok = ft->GetOpenCommand(&cmd, wxFileType::MessageParameters(params.url));
         delete ft;
     }
 #endif // wxUSE_MIMETYPE
@@ -1053,7 +1051,7 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
     {
         // fallback to checking for the BROWSER environment variable
         if ( !wxGetEnv(wxT("BROWSER"), &cmd) || cmd.empty() )
-            cmd << wxT(' ') << url;
+            cmd << wxT(' ') << params.url;
     }
 
     ok = ( !cmd.empty() && wxExecute(cmd) );
@@ -1067,90 +1065,54 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
 }
 #endif
 
-static bool DoLaunchDefaultBrowserHelper(const wxString& urlOrig, int flags)
+static bool DoLaunchDefaultBrowserHelper(const wxString& url, int flags)
 {
-    // NOTE: we don't have to care about the wxBROWSER_NOBUSYCURSOR flag
+    wxLaunchBrowserParams params(flags);
-    //       as it was already handled by wxLaunchDefaultBrowser
 
-    wxUnusedVar(flags);
+    const wxURI uri(url);
-
-    wxString url(urlOrig), scheme;
-    wxURI uri(url);
 
     // this check is useful to avoid that wxURI recognizes as scheme parts of
-    // the filename, in case urlOrig is a local filename
+    // the filename, in case url is a local filename
     // (e.g. "C:\\test.txt" when parsed by wxURI reports a scheme == "C")
     bool hasValidScheme = uri.HasScheme() && uri.GetScheme().length() > 1;
 
-#if defined(__WINDOWS__)
-
-    // NOTE: when testing wxMSW's wxLaunchDefaultBrowser all possible forms
-    //       of the URL/flags should be tested; e.g.:
-    //
-    // for (int i=0; i<2; i++)
-    // {
-    //   // test arguments without a valid URL scheme:
-    //   wxLaunchDefaultBrowser("C:\\test.txt", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
-    //   wxLaunchDefaultBrowser("wxwidgets.org", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
-    //
-    //   // test arguments with different valid schemes:
-    //   wxLaunchDefaultBrowser("file:/C%3A/test.txt", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
-    //   wxLaunchDefaultBrowser("http://wxwidgets.org", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
-    //   wxLaunchDefaultBrowser("mailto:user@host.org", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
-    // }
-    // (assuming you have a C:\test.txt file)
-
     if ( !hasValidScheme )
     {
-        if (wxFileExists(urlOrig) || wxDirExists(urlOrig))
+        if (wxFileExists(url) || wxDirExists(url))
         {
-            scheme = "file";
+            params.scheme = "file";
-            // do not prepend the file scheme to the URL as ShellExecuteEx() doesn't like it
+            params.path = url;
         }
         else
         {
-            url.Prepend(wxS("http://"));
+            params.scheme = "http";
-            scheme = "http";
         }
+
+        params.url << params.scheme << wxS("://") << url;
     }
     else if ( hasValidScheme )
     {
-        scheme = uri.GetScheme();
+        params.url = url;
+        params.scheme = uri.GetScheme();
 
-        if ( uri.GetScheme() == "file" )
+        if ( params.scheme == "file" )
         {
             // TODO: extract URLToFileName() to some always compiled in
             //       function
 #if wxUSE_FILESYSTEM
-            // ShellExecuteEx() doesn't like the "file" scheme when opening local files;
+            // for same reason as above, remove the scheme from the URL
-            // remove it
+            params.path = wxFileSystem::URLToFileName(url).GetFullPath();
-            url = wxFileSystem::URLToFileName(url).GetFullPath();
 #endif // wxUSE_FILESYSTEM
         }
     }
 
-    if (wxDoLaunchDefaultBrowser(url, scheme, flags))
+    if ( !wxDoLaunchDefaultBrowser(params) )
-        return true;
-    //else: call wxLogSysError
-#else
-    if ( !hasValidScheme )
     {
-        // set the scheme of url to "http" or "file" if it does not have one
+        wxLogSysError(_("Failed to open URL \"%s\" in default browser."), url);
-        if (wxFileExists(urlOrig) || wxDirExists(urlOrig))
+        return false;
-            url.Prepend(wxS("file://"));
-        else
-            url.Prepend(wxS("http://"));
     }
 
-    if (wxDoLaunchDefaultBrowser(url, flags))
+    return true;
-        return true;
-    //else: call wxLogSysError
-#endif
-
-    wxLogSysError(_("Failed to open URL \"%s\" in default browser."),
-                  url.c_str());
-
-    return false;
 }
 
 bool wxLaunchDefaultBrowser(const wxString& url, int flags)

src/msw/utilswin.cpp

@@ -18,6 +18,7 @@
     #include "wx/utils.h"
 #endif //WX_PRECOMP
 
+#include "wx/private/launchbrowser.h"
 #include "wx/msw/private.h"     // includes <windows.h>
 #include "wx/msw/registry.h"
 #include <shellapi.h> // needed for SHELLEXECUTEINFO
@@ -49,16 +50,30 @@ bool wxLaunchDefaultApplication(const wxString& document, int flags)
 // Launch default browser
 // ----------------------------------------------------------------------------
 
-bool wxDoLaunchDefaultBrowser(const wxString& url, const wxString& scheme, int flags)
+// NOTE: when testing wxMSW's wxLaunchDefaultBrowser all possible forms
-{
+//       of the URL/flags should be tested; e.g.:
-    wxUnusedVar(flags);
+//
+// for (int i=0; i<2; i++)
+// {
+//   // test arguments without a valid URL scheme:
+//   wxLaunchDefaultBrowser("C:\\test.txt", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
+//   wxLaunchDefaultBrowser("wxwidgets.org", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
+//
+//   // test arguments with different valid schemes:
+//   wxLaunchDefaultBrowser("file:/C%3A/test.txt", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
+//   wxLaunchDefaultBrowser("http://wxwidgets.org", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
+//   wxLaunchDefaultBrowser("mailto:user@host.org", i==0 ? 0 : wxBROWSER_NEW_WINDOW);
+// }
+// (assuming you have a C:\test.txt file)
 
+bool wxDoLaunchDefaultBrowser(const wxLaunchBrowserParams& params)
+{
 #if wxUSE_IPC
-    if ( flags & wxBROWSER_NEW_WINDOW )
+    if ( params.flags & wxBROWSER_NEW_WINDOW )
     {
         // ShellExecuteEx() opens the URL in an existing window by default so
         // we can't use it if we need a new window
-        wxRegKey key(wxRegKey::HKCR, scheme + wxT("\\shell\\open"));
+        wxRegKey key(wxRegKey::HKCR, params.scheme + wxT("\\shell\\open"));
         if ( !key.Exists() )
         {
             // try the default browser, it must be registered at least for http URLs
@@ -98,7 +113,7 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, const wxString& scheme, int f
                     // contain a placeholder for the URL and we should fail if
                     // we didn't find it as this would mean that we have no way
                     // of passing the URL to the browser
-                    ok = ddeCmd.Replace(wxT("%1"), url, false) == 1;
+                    ok = ddeCmd.Replace(wxT("%1"), params.url, false) == 1;
                 }
 
                 if ( ok )
@@ -122,7 +137,7 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, const wxString& scheme, int f
 #endif // wxUSE_IPC
 
     WinStruct<SHELLEXECUTEINFO> sei;
-    sei.lpFile = url.c_str();
+    sei.lpFile = params.GetPathOrURL().t_str();
     sei.lpVerb = wxT("open");
     sei.nShow = SW_SHOWNORMAL;
     sei.fMask = SEE_MASK_FLAG_NO_UI; // we give error message ourselves

src/osx/iphone/utils.mm

@@ -28,6 +28,7 @@
 #include "wx/osx/private.h"
 
 #if wxUSE_GUI
+    #include "wx/private/launchbrowser.h"
     #include "wx/osx/private/timer.h"
     #include "wx/osx/dcclient.h"
 #endif // wxUSE_GUI
@@ -102,9 +103,9 @@ void wxBell()
 // Launch default browser
 // ----------------------------------------------------------------------------
 
-bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
+bool wxDoLaunchDefaultBrowser(const wxLaunchBrowserParams& params)
 {
-    return [[UIApplication sharedApplication] openURL:[NSURL URLWithString:wxCFStringRef(url).AsNSString()]]
+    return [[UIApplication sharedApplication] openURL:[NSURL URLWithString:wxCFStringRef(params.url).AsNSString()]]
         == YES;
 }
 

src/osx/utils_osx.cpp

@@ -36,6 +36,10 @@
 
 #include <AudioToolbox/AudioServices.h>
 
+#if wxUSE_GUI
+    #include "wx/private/launchbrowser.h"
+#endif
+
 #include "wx/osx/private.h"
 #include "wx/osx/private/timer.h"
 
@@ -130,11 +134,10 @@ bool wxLaunchDefaultApplication(const wxString& document, int flags)
 // Launch default browser
 // ----------------------------------------------------------------------------
 
-bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
+bool wxDoLaunchDefaultBrowser(const wxLaunchBrowserParams& params)
 {
-    wxUnusedVar(flags);
     wxCFRef< CFURLRef > curl( CFURLCreateWithString( kCFAllocatorDefault,
-                              wxCFStringRef( url ), NULL ) );
+                              wxCFStringRef( params.url ), NULL ) );
     OSStatus err = LSOpenCFURLRef( curl , NULL );
 
     if (err == noErr)

src/unix/utilsx11.cpp

@@ -24,6 +24,7 @@
 
 #include "wx/iconbndl.h"
 #include "wx/apptrait.h"
+#include "wx/private/launchbrowser.h"
 
 #ifdef __VMS
 #pragma message disable nosimpint
@@ -2609,10 +2610,9 @@ bool wxLaunchDefaultApplication(const wxString& document, int flags)
 // Launch default browser
 // ----------------------------------------------------------------------------
 
-bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
+bool
+wxDoLaunchDefaultBrowser(const wxLaunchBrowserParams& params)
 {
-    wxUnusedVar(flags);
-
 #ifdef __WXGTK__
 #if GTK_CHECK_VERSION(2,14,0)
 #ifndef __WXGTK3__
@@ -2620,7 +2620,7 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
 #endif
     {
         GdkScreen* screen = gdk_window_get_screen(wxGetTopLevelGDK());
-        if (gtk_show_uri(screen, url.utf8_str(), GDK_CURRENT_TIME, NULL))
+        if (gtk_show_uri(screen, params.url.utf8_str(), GDK_CURRENT_TIME, NULL))
             return true;
     }
 #endif // GTK_CHECK_VERSION(2,14,0)
@@ -2636,7 +2636,7 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
     if ( wxGetEnv("PATH", &path) &&
          wxFindFileInPath(&xdg_open, path, "xdg-open") )
     {
-        if ( wxExecute(xdg_open + " " + url) )
+        if ( wxExecute(xdg_open + " " + params.GetPathOrURL()) )
             return true;
     }
 
@@ -2655,7 +2655,7 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
         if (res >= 0 && errors.GetCount() == 0)
         {
             wxString cmd = output[0];
-            cmd << wxT(' ') << url;
+            cmd << wxT(' ') << params.GetPathOrURL();
             if (wxExecute(cmd))
                 return true;
         }
@@ -2663,7 +2663,7 @@ bool wxDoLaunchDefaultBrowser(const wxString& url, int flags)
     else if (desktop == wxT("KDE"))
     {
         // kfmclient directly opens the given URL
-        if (wxExecute(wxT("kfmclient openURL ") + url))
+        if (wxExecute(wxT("kfmclient openURL ") + params.GetPathOrURL()))
             return true;
     }