Fix handling of single letter shares in UNC paths in wxFileName

This comes at the price of breaking compatibility and returning
"\\share" rather than just "share" from wxFileName::GetVolume() for the
UNC paths. This breakage seems justified because it is required in order
to allow application code to distinguish between paths "x:\foo" and
"\\x\foo", which was previously impossible as GetVolume() returned just
"x" in both cases.

Document this change, adjust the existing checks for the new GetVolume()
semantics and add a new test which passes now, but didn't pass before.

Closes #19255.

This commit is best viewed ignoring whitespace-only changes.

docs/changes.txt

@@ -133,6 +133,12 @@ Changes in behaviour not resulting in compilation errors
   bitmaps in wxMSW if the corresponding Set had been called before, as in the
   other ports, instead of returning the normal bitmap as fallback in this case.
 
+- wxFileName::GetVolume() now returns "\\share" and not just "share" for the
+  UNC paths (i.e. \\share\path\on\remote\server) and "\\?\Volume{GUID}" for the
+  volume GUID paths rather than just "Volume{GUID}" as before. This allows
+  distinguishing them from the drive letters, even for single letter network
+  share name.
+
 
 Changes in behaviour which may result in build errors
 -----------------------------------------------------

include/wx/filename.h

@@ -638,6 +638,9 @@ private:
                    int flags = SetPath_MayHaveVolume);
 
     // the drive/volume/device specification (always empty for Unix)
+    //
+    // for the drive letters, contains just the letter itself, but for MSW UNC
+    // and volume GUID paths, it starts with double backslash, e.g. "\\share"
     wxString        m_volume;
 
     // the path components of the file

interface/wx/filename.h

@@ -859,9 +859,19 @@ public:
                   wxDateTime* dtCreate) const;
 
     /**
-        Returns the string containing the volume for this file name, empty if it
+        Returns the string containing the volume for this file name.
-        doesn't have one or if the file system doesn't support volumes at all
+
-        (for example, Unix).
+        The returned string is empty if this object doesn't have a volume name,
+        as is always the case for the paths in Unix format which don't support
+        volumes at all.
+
+        Note that for @c wxPATH_DOS format paths, the returned string may have
+        one of the following forms:
+
+        - Just a single letter, for the usual drive letter volumes, e.g. @c C.
+        - A share name preceded by a double backslash, e.g. @c \\\\share.
+        - A GUID volume preceded by a double backslash and a question mark,
+          e.g. @c \\\\?\\Volume{12345678-9abc-def0-1234-56789abcdef0}.
     */
     wxString GetVolume() const;
 

src/common/filename.cpp

@@ -133,6 +133,13 @@ extern const wxULongLong wxInvalidSize = (unsigned)-1;
 namespace
 {
 
+// ----------------------------------------------------------------------------
+// private constants
+// ----------------------------------------------------------------------------
+
+// length of \\?\Volume{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}\ string
+static const size_t wxMSWUniqueVolumePrefixLength = 49;
+
 // ----------------------------------------------------------------------------
 // private classes
 // ----------------------------------------------------------------------------
@@ -250,30 +257,31 @@ static wxString wxGetVolumeString(const wxString& volume, wxPathFormat format)
     {
         format = wxFileName::GetFormat(format);
 
-        // Special Windows UNC paths hack, part 2: undo what we did in
+        switch ( format )
-        // SplitPath() and make an UNC path if we have a drive which is not a
-        // single letter (hopefully the network shares can't be one letter only
-        // although I didn't find any authoritative docs on this)
-        if ( format == wxPATH_DOS && volume.length() > 1 )
         {
-            // We also have to check for Windows unique volume names here and
+            case wxPATH_DOS:
-            // return it with '\\?\' prepended to it
+                path = volume;
-            if ( wxFileName::IsMSWUniqueVolumeNamePath("\\\\?\\" + volume + "\\",
+
-                                                       format) )
+                // We shouldn't use a colon after the volume in UNC and volume
-            {
+                // GUID paths, so append it only if it's just a drive letter.
-                path << "\\\\?\\" << volume;
+                if ( volume.length() == 1 )
-            }
+                    path += wxFileName::GetVolumeSeparator(format);
-            else
+                break;
-            {
+
-                // it must be a UNC path
+            case wxPATH_VMS:
-                path << wxFILE_SEP_PATH_DOS << wxFILE_SEP_PATH_DOS << volume;
+                path << volume << wxFileName::GetVolumeSeparator(format);
-            }
+                break;
+
+            case wxPATH_MAC:
+            case wxPATH_UNIX:
+                // Volumes are not used in paths in this format.
+                break;
+
+            case wxPATH_NATIVE:
+            case wxPATH_MAX:
+                wxFAIL_MSG( wxS("unreachable") );
+                break;
         }
-        else if  ( format == wxPATH_DOS || format == wxPATH_VMS )
-        {
-            path << volume << wxFileName::GetVolumeSeparator(format);
-        }
-        // else ignore
     }
 
     return path;
@@ -286,17 +294,23 @@ inline bool IsDOSPathSep(wxUniChar ch)
     return ch == wxFILE_SEP_PATH_DOS || ch == wxFILE_SEP_PATH_UNIX;
 }
 
-// return true if the format used is the DOS/Windows one and the string looks
+// return true if the string looks like a UNC path
-// like a UNC path
+static bool IsUNCPath(const wxString& path)
-static bool IsUNCPath(const wxString& path, wxPathFormat format)
 {
-    return wxFileName::GetFormat(format) == wxPATH_DOS &&
+    return path.length() >= 3 && // "\\a" is the shortest UNC path
-                path.length() >= 4 && // "\\a" can't be a UNC path
                     IsDOSPathSep(path[0u]) &&
                         IsDOSPathSep(path[1u]) &&
                             !IsDOSPathSep(path[2u]);
 }
 
+// return true if the string looks like a GUID volume path ("\\?\Volume{guid}\")
+static bool IsVolumeGUIDPath(const wxString& path)
+{
+    return path.length() >= wxMSWUniqueVolumePrefixLength &&
+             path.StartsWith(wxS("\\\\?\\Volume{")) &&
+              path[wxMSWUniqueVolumePrefixLength - 1] == wxFILE_SEP_PATH_DOS;
+}
+
 // Under Unix-ish systems (basically everything except Windows but we can't
 // just test for non-__WIN32__ because Cygwin defines it, yet we want to use
 // lstat() under it, so test for all the rest explicitly) we may work either
@@ -353,13 +367,6 @@ bool StatAny(wxStructStat& st, const wxFileName& fn)
 
 #endif // wxHAVE_LSTAT
 
-// ----------------------------------------------------------------------------
-// private constants
-// ----------------------------------------------------------------------------
-
-// length of \\?\Volume{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}\ string
-static const size_t wxMSWUniqueVolumePrefixLength = 49;
-
 } // anonymous namespace
 
 // ============================================================================
@@ -1986,10 +1993,7 @@ wxFileName::IsMSWUniqueVolumeNamePath(const wxString& path, wxPathFormat format)
 {
     // return true if the format used is the DOS/Windows one and the string begins
     // with a Windows unique volume name ("\\?\Volume{guid}\")
-    return format == wxPATH_DOS &&
+    return GetFormat(format) == wxPATH_DOS && IsVolumeGUIDPath(path);
-            path.length() >= wxMSWUniqueVolumePrefixLength &&
-             path.StartsWith(wxS("\\\\?\\Volume{")) &&
-              path[wxMSWUniqueVolumePrefixLength - 1] == wxFILE_SEP_PATH_DOS;
 }
 
 // ----------------------------------------------------------------------------
@@ -2332,71 +2336,100 @@ wxString wxFileName::GetVolumeString(char drive, int flags)
 
 /* static */
 void
-wxFileName::SplitVolume(const wxString& fullpathWithVolume,
+wxFileName::SplitVolume(const wxString& fullpath,
                         wxString *pstrVolume,
                         wxString *pstrPath,
                         wxPathFormat format)
 {
     format = GetFormat(format);
 
-    wxString fullpath = fullpathWithVolume;
+    wxString pathOnly;
 
-    if ( IsMSWUniqueVolumeNamePath(fullpath, format) )
+    switch ( format )
     {
-        // special Windows unique volume names hack: transform
+        case wxPATH_DOS:
-        // \\?\Volume{guid}\path into Volume{guid}:path
+            // Deal with MSW UNC and volume GUID paths complications first.
-        // note: this check must be done before the check for UNC path
+            if ( IsVolumeGUIDPath(fullpath) )
-
-        // we know the last backslash from the unique volume name is located
-        // there from IsMSWUniqueVolumeNamePath
-        fullpath[wxMSWUniqueVolumePrefixLength - 1] = wxFILE_SEP_DSK;
-
-        // paths starting with a unique volume name should always be absolute
-        fullpath.insert(wxMSWUniqueVolumePrefixLength, 1, wxFILE_SEP_PATH_DOS);
-
-        // remove the leading "\\?\" part
-        fullpath.erase(0, 4);
-    }
-    else if ( IsUNCPath(fullpath, format) )
-    {
-        // special Windows UNC paths hack: transform \\share\path into share:path
-
-        fullpath.erase(0, 2);
-
-        size_t posFirstSlash =
-            fullpath.find_first_of(GetPathTerminators(format));
-        if ( posFirstSlash != wxString::npos )
-        {
-            fullpath[posFirstSlash] = wxFILE_SEP_DSK;
-
-            // UNC paths are always absolute, right? (FIXME)
-            fullpath.insert(posFirstSlash + 1, 1, wxFILE_SEP_PATH_DOS);
-        }
-    }
-
-    // We separate the volume here
-    if ( format == wxPATH_DOS || format == wxPATH_VMS )
-    {
-        wxString sepVol = GetVolumeSeparator(format);
-
-        // we have to exclude the case of a colon in the very beginning of the
-        // string as it can't be a volume separator (nor can this be a valid
-        // DOS file name at all but we'll leave dealing with this to our caller)
-        size_t posFirstColon = fullpath.find_first_of(sepVol);
-        if ( posFirstColon && posFirstColon != wxString::npos )
-        {
-            if ( pstrVolume )
             {
-                *pstrVolume = fullpath.Left(posFirstColon);
+                if ( pstrVolume )
+                    *pstrVolume = fullpath.Left(wxMSWUniqueVolumePrefixLength - 1);
+
+                // Note: take the first slash here.
+                pathOnly = fullpath.Mid(wxMSWUniqueVolumePrefixLength - 1);
+
+                break;
             }
 
-            // remove the volume name and the separator from the full path
+            if ( IsUNCPath(fullpath) )
-            fullpath.erase(0, posFirstColon + sepVol.length());
+            {
-        }
+                // Note that IsUNCPath() checks that 3rd character is not a
+                // (back)slash.
+                size_t posFirstSlash =
+                    fullpath.find_first_of(GetPathTerminators(format), 3);
+                if ( posFirstSlash != wxString::npos )
+                {
+                    if ( pstrVolume )
+                        *pstrVolume = fullpath.Left(posFirstSlash);
+                    pathOnly = fullpath.Mid(posFirstSlash);
+                }
+                else // UNC path to the root of the share (just "\\share")
+                {
+                    if ( pstrVolume )
+                        *pstrVolume = fullpath;
+                }
+
+                // In any case, normalize slashes to backslashes, which are canonical
+                // separators for the UNC paths.
+                if ( pstrVolume )
+                {
+                    (*pstrVolume)[0] =
+                    (*pstrVolume)[1] = '\\';
+                }
+
+                break;
+            }
+
+            wxFALLTHROUGH;
+
+        case wxPATH_VMS:
+            {
+                wxString sepVol = GetVolumeSeparator(format);
+
+                // we have to exclude the case of a colon in the very beginning of the
+                // string as it can't be a volume separator (nor can this be a valid
+                // DOS file name at all but we'll leave dealing with this to our caller)
+                size_t posFirstColon = fullpath.find_first_of(sepVol);
+                if ( posFirstColon && posFirstColon != wxString::npos )
+                {
+                    if ( pstrVolume )
+                    {
+                        *pstrVolume = fullpath.Left(posFirstColon);
+                    }
+
+                    // remove the volume name and the separator from the full path
+                    pathOnly = fullpath.Mid(posFirstColon + 1);
+                }
+                else
+                {
+                    pathOnly = fullpath;
+                }
+            }
+            break;
+
+        case wxPATH_MAC:
+        case wxPATH_UNIX:
+            // Volumes are not used in paths in this format.
+            pathOnly = fullpath;
+            break;
+
+        case wxPATH_NATIVE:
+        case wxPATH_MAX:
+            wxFAIL_MSG( wxS("unreachable") );
+            break;
     }
 
     if ( pstrPath )
-        *pstrPath = fullpath;
+        *pstrPath = pathOnly;
 }
 
 /* static */

tests/filename/filenametest.cpp

@@ -82,9 +82,9 @@ static struct TestFileNameInfo
     { "c:\\foo.bar", "c", "\\", "foo", "bar", true, wxPATH_DOS },
     { "c:\\Windows\\command.com", "c", "\\Windows", "command", "com", true, wxPATH_DOS },
     { "\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}\\",
-      "Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}", "\\", "", "", true, wxPATH_DOS },
+      "\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}", "\\", "", "", true, wxPATH_DOS },
     { "\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}\\Program Files\\setup.exe",
-      "Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}", "\\Program Files", "setup", "exe", true, wxPATH_DOS },
+      "\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}", "\\Program Files", "setup", "exe", true, wxPATH_DOS },
 
 #if 0
     // NB: when using the wxFileName::GetLongPath() function on these two
@@ -544,11 +544,11 @@ TEST_CASE("wxFileName::ShortLongPath", "[filename]")
 TEST_CASE("wxFileName::UNC", "[filename]")
 {
     wxFileName fn("//share/path/name.ext", wxPATH_DOS);
-    CHECK( fn.GetVolume() == "share" );
+    CHECK( fn.GetVolume() == "\\\\share" );
     CHECK( fn.GetPath(wxPATH_NO_SEPARATOR, wxPATH_DOS) == "\\path" );
 
     fn.Assign("\\\\share2\\path2\\name.ext", wxPATH_DOS);
-    CHECK( fn.GetVolume() == "share2" );
+    CHECK( fn.GetVolume() == "\\\\share2" );
     CHECK( fn.GetPath(wxPATH_NO_SEPARATOR, wxPATH_DOS) == "\\path2" );
 
 #ifdef __WINDOWS__
@@ -557,6 +557,11 @@ TEST_CASE("wxFileName::UNC", "[filename]")
     // beginning.
     fn.Assign("d:\\\\root\\directory\\file");
     CHECK( fn.GetFullPath() == "d:\\root\\directory\\file" );
+
+    // Check that single letter UNC paths don't turn into drive letters, as
+    // they used to do.
+    fn.Assign("\\\\x\\dir\\file");
+    CHECK( fn.GetFullPath() == "\\\\x\\dir\\file" );
 #endif // __WINDOWS__
 }
 
@@ -564,13 +569,13 @@ TEST_CASE("wxFileName::VolumeUniqueName", "[filename]")
 {
     wxFileName fn("\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}\\",
                   wxPATH_DOS);
-    CHECK( fn.GetVolume() == "Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}" );
+    CHECK( fn.GetVolume() == "\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}" );
     CHECK( fn.GetPath(wxPATH_NO_SEPARATOR, wxPATH_DOS) == "\\" );
     CHECK( fn.GetFullPath(wxPATH_DOS) == "\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}\\" );
 
     fn.Assign("\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}\\"
               "Program Files\\setup.exe", wxPATH_DOS);
-    CHECK( fn.GetVolume() == "Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}" );
+    CHECK( fn.GetVolume() == "\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}" );
     CHECK( fn.GetPath(wxPATH_NO_SEPARATOR, wxPATH_DOS) == "\\Program Files" );
     CHECK( fn.GetFullPath(wxPATH_DOS) == "\\\\?\\Volume{8089d7d7-d0ac-11db-9dd0-806d6172696f}\\Program Files\\setup.exe" );
 }