From 8db2f4517127f0ce2822a896328d1b398e850bd3 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 13 Sep 2021 20:53:27 +0100 Subject: [PATCH 1/7] Fix handling multiple leading backslashes in wxFileName under MSW Extend the existing workaround to work not only with explicitly wxPATH_DOS paths, but with paths implicitly using the DOS format due to it being the default format for the current platform. Closes #19261. --- src/common/filename.cpp | 2 +- tests/filename/filenametest.cpp | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/common/filename.cpp b/src/common/filename.cpp index d31d3a5f35..1a40f08a10 100644 --- a/src/common/filename.cpp +++ b/src/common/filename.cpp @@ -290,7 +290,7 @@ inline bool IsDOSPathSep(wxUniChar ch) // like a UNC path static bool IsUNCPath(const wxString& path, wxPathFormat format) { - return format == wxPATH_DOS && + return wxFileName::GetFormat(format) == wxPATH_DOS && path.length() >= 4 && // "\\a" can't be a UNC path IsDOSPathSep(path[0u]) && IsDOSPathSep(path[1u]) && diff --git a/tests/filename/filenametest.cpp b/tests/filename/filenametest.cpp index 3134bbe53d..ba8a326a77 100644 --- a/tests/filename/filenametest.cpp +++ b/tests/filename/filenametest.cpp @@ -550,6 +550,14 @@ TEST_CASE("wxFileName::UNC", "[filename]") fn.Assign("\\\\share2\\path2\\name.ext", wxPATH_DOS); CHECK( fn.GetVolume() == "share2" ); CHECK( fn.GetPath(wxPATH_NO_SEPARATOR, wxPATH_DOS) == "\\path2" ); + +#ifdef __WINDOWS__ + // Check that doubled backslashes in the beginning of the path are not + // misinterpreted as UNC volume when we have a drive letter in the + // beginning. + fn.Assign("d:\\\\root\\directory\\file"); + CHECK( fn.GetFullPath() == "d:\\root\\directory\\file" ); +#endif // __WINDOWS__ } TEST_CASE("wxFileName::VolumeUniqueName", "[filename]") From 8d13440d69bb4f1283f528cf8f6ee35b08555c73 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 13 Sep 2021 21:05:21 +0100 Subject: [PATCH 2/7] Fix wxPATH_RMDIR_FULL documentation Explain what this flag really does in Rmdir() documentation itself, as the old wording was confusing. Closes #16682. --- interface/wx/filename.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/interface/wx/filename.h b/interface/wx/filename.h index 7dc34abdac..1eed1b4b0d 100644 --- a/interface/wx/filename.h +++ b/interface/wx/filename.h @@ -1225,9 +1225,12 @@ public: Deletes the specified directory from the file system. @param flags - Can contain one of wxPATH_RMDIR_FULL or wxPATH_RMDIR_RECURSIVE. By - default contains neither so the directory will not be removed - unless it is empty. + With default value, the directory is removed only if it is empty. + If wxPATH_RMDIR_FULL is specified, it is removed even if it + contains subdirectories, provided that there are no files in + neither this directory nor its subdirectories. If flags contains + wxPATH_RMDIR_RECURSIVE, then the directory is removed with all the + files and directories under it. @return Returns @true if the directory was successfully deleted, @false otherwise. @@ -1240,9 +1243,12 @@ public: @param dir The directory to delete @param flags - Can contain one of wxPATH_RMDIR_FULL or wxPATH_RMDIR_RECURSIVE. By - default contains neither so the directory will not be removed - unless it is empty. + With default value, the directory is removed only if it is empty. + If wxPATH_RMDIR_FULL is specified, it is removed even if it + contains subdirectories, provided that there are no files in + neither this directory nor its subdirectories. If flags contains + wxPATH_RMDIR_RECURSIVE, then the directory is removed with all the + files and directories under it. @return Returns @true if the directory was successfully deleted, @false otherwise. From abf26b03a79ae8b13c1ce7da889a7ef934d52244 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 13 Sep 2021 21:25:30 +0100 Subject: [PATCH 3/7] Fix wxFileName::Mkdir("/foo") under MSW Creating a directory with the leading path separator unexpectedly created it under the current directory rather than in the root of the current drive under MSW, due to the path being considered relative, in spite of starting with the path separator, because it didn't have the volume. Fix this by avoiding the use of IsAbsolute() in Mkdir() and checking for m_relative instead, as it seems the safest possible fix for this bug because changing IsAbsolute() to return true in this case might change the behaviour of the existing code. Closes #4470. --- src/common/filename.cpp | 5 ++++- tests/filename/filenametest.cpp | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/common/filename.cpp b/src/common/filename.cpp index 1a40f08a10..3f811c17d3 100644 --- a/src/common/filename.cpp +++ b/src/common/filename.cpp @@ -1283,7 +1283,10 @@ bool wxFileName::Mkdir( const wxString& dir, int perm, int flags ) size_t count = dirs.GetCount(); for ( size_t i = 0; i < count; i++ ) { - if ( i > 0 || filename.IsAbsolute() ) + // Do not use IsAbsolute() here because we want the path to start + // with the separator even if it doesn't have any volume, but + // IsAbsolute() would return false in this case. + if ( i > 0 || !filename.m_relative ) currPath += wxFILE_SEP_PATH; currPath += dirs[i]; diff --git a/tests/filename/filenametest.cpp b/tests/filename/filenametest.cpp index ba8a326a77..75035cf552 100644 --- a/tests/filename/filenametest.cpp +++ b/tests/filename/filenametest.cpp @@ -716,6 +716,18 @@ TEST_CASE("wxFileName::Exists", "[filename]") #endif // __UNIX__ } +TEST_CASE("wxFileName::Mkdir", "[filename]") +{ + wxFileName fn; + fn.AssignDir("/foo/bar"); + if ( fn.Mkdir(wxS_DIR_DEFAULT, wxPATH_MKDIR_FULL) ) + { + CHECK( fn.DirExists() ); + CHECK( fn.Rmdir() ); + } + //else: creating the directory may fail because of permissions +} + TEST_CASE("wxFileName::SameAs", "[filename]") { wxFileName fn1( wxFileName::CreateTempFileName( "filenametest1" ) ); From 7e4b54a00acdf53ef47cec3e64fc57180e09262b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 15 Sep 2021 00:35:34 +0100 Subject: [PATCH 4/7] Replace old hack in wxFileName::Assign() with better solution Replace the workaround introduced as a "temporary fix to avoid breaking backwards compatibility in 2.8" back in 9e1c7236e0 (don't treat foo in c:\\foo\bar as network share, 2006-12-17) with a better solution proposed in the comment in that commit, i.e. don't even try to extract the volume from the path if we already have the volume separately. This commit is best viewed ignoring whitespace-only changes. --- include/wx/filename.h | 11 ++++++++ src/common/filename.cpp | 56 ++++++++++++++++++++--------------------- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/include/wx/filename.h b/include/wx/filename.h index f592996548..490a95cd92 100644 --- a/include/wx/filename.h +++ b/include/wx/filename.h @@ -626,6 +626,17 @@ private: // check whether this dir is valid for Append/Prepend/InsertDir() static bool IsValidDirComponent(const wxString& dir); + // flags used with DoSetPath() + enum + { + SetPath_PathOnly = 0, + SetPath_MayHaveVolume = 1 + }; + + // helper of public SetPath() also used internally + void DoSetPath(const wxString& path, wxPathFormat format, + int flags = SetPath_MayHaveVolume); + // the drive/volume/device specification (always empty for Unix) wxString m_volume; diff --git a/src/common/filename.cpp b/src/common/filename.cpp index 3f811c17d3..20425262bc 100644 --- a/src/common/filename.cpp +++ b/src/common/filename.cpp @@ -392,22 +392,7 @@ void wxFileName::Assign(const wxString& volume, // have the volume here and the UNC notation (\\server\path) is only valid // for paths which don't start with a volume, so prevent SetPath() from // recognizing "\\foo\bar" in "c:\\foo\bar" as an UNC path - // - // note also that this is a rather ugly way to do what we want (passing - // some kind of flag telling to ignore UNC paths to SetPath() would be - // better) but this is the safest thing to do to avoid breaking backwards - // compatibility in 2.8 - if ( IsUNCPath(path, format) ) - { - // remove one of the 2 leading backslashes to ensure that it's not - // recognized as an UNC path by SetPath() - wxString pathNonUNC(path, 1, wxString::npos); - SetPath(pathNonUNC, format); - } - else // no UNC complications - { - SetPath(path, format); - } + DoSetPath(path, format, SetPath_PathOnly); m_volume = volume; m_ext = ext; @@ -416,7 +401,13 @@ void wxFileName::Assign(const wxString& volume, m_hasExt = hasExt; } -void wxFileName::SetPath( const wxString& pathOrig, wxPathFormat format ) +void wxFileName::SetPath( const wxString& path, wxPathFormat format ) +{ + DoSetPath(path, format, SetPath_MayHaveVolume); +} + +void +wxFileName::DoSetPath(const wxString& pathOrig, wxPathFormat format, int flags) { m_dirs.Clear(); @@ -431,24 +422,31 @@ void wxFileName::SetPath( const wxString& pathOrig, wxPathFormat format ) format = GetFormat( format ); // 0) deal with possible volume part first - wxString volume, - path; - SplitVolume(pathOrig, &volume, &path, format); - if ( !volume.empty() ) + wxString path; + if ( flags & SetPath_MayHaveVolume ) { - m_relative = false; + wxString volume; + SplitVolume(pathOrig, &volume, &path, format); + if ( !volume.empty() ) + { + m_relative = false; - SetVolume(volume); + SetVolume(volume); + } + + if ( path.empty() ) + { + // we had only the volume + return; + } + } + else + { + path = pathOrig; } // 1) Determine if the path is relative or absolute. - if ( path.empty() ) - { - // we had only the volume - return; - } - wxChar leadingChar = path[0u]; switch (format) From 549e0a59b1157e8606d70c1ed4192676d7080e1a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 15 Sep 2021 01:51:35 +0100 Subject: [PATCH 5/7] 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 | 6 + include/wx/filename.h | 3 + interface/wx/filename.h | 16 ++- src/common/filename.cpp | 211 ++++++++++++++++++-------------- tests/filename/filenametest.cpp | 17 ++- 5 files changed, 155 insertions(+), 98 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 7653c3a2f5..a1b8c69426 100644 --- a/docs/changes.txt +++ b/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 ----------------------------------------------------- diff --git a/include/wx/filename.h b/include/wx/filename.h index 490a95cd92..003674df85 100644 --- a/include/wx/filename.h +++ b/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 diff --git a/interface/wx/filename.h b/interface/wx/filename.h index 1eed1b4b0d..d8ffcb16e7 100644 --- a/interface/wx/filename.h +++ b/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 - doesn't have one or if the file system doesn't support volumes at all - (for example, Unix). + Returns the string containing the volume for this file name. + + 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; diff --git a/src/common/filename.cpp b/src/common/filename.cpp index 20425262bc..a57816daf0 100644 --- a/src/common/filename.cpp +++ b/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 - // 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 ) + switch ( format ) { - // We also have to check for Windows unique volume names here and - // return it with '\\?\' prepended to it - if ( wxFileName::IsMSWUniqueVolumeNamePath("\\\\?\\" + volume + "\\", - format) ) - { - path << "\\\\?\\" << volume; - } - else - { - // it must be a UNC path - path << wxFILE_SEP_PATH_DOS << wxFILE_SEP_PATH_DOS << volume; - } + case wxPATH_DOS: + path = volume; + + // 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. + if ( volume.length() == 1 ) + path += wxFileName::GetVolumeSeparator(format); + break; + + case wxPATH_VMS: + 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 -// like a UNC path -static bool IsUNCPath(const wxString& path, wxPathFormat format) +// return true if the string looks like a UNC path +static bool IsUNCPath(const wxString& path) { - return wxFileName::GetFormat(format) == wxPATH_DOS && - path.length() >= 4 && // "\\a" can't be a UNC path + return path.length() >= 3 && // "\\a" is the shortest 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 && - path.length() >= wxMSWUniqueVolumePrefixLength && - path.StartsWith(wxS("\\\\?\\Volume{")) && - path[wxMSWUniqueVolumePrefixLength - 1] == wxFILE_SEP_PATH_DOS; + return GetFormat(format) == wxPATH_DOS && IsVolumeGUIDPath(path); } // ---------------------------------------------------------------------------- @@ -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 - // \\?\Volume{guid}\path into Volume{guid}:path - // note: this check must be done before the check for UNC path - - // 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 ) + case wxPATH_DOS: + // Deal with MSW UNC and volume GUID paths complications first. + if ( IsVolumeGUIDPath(fullpath) ) { - *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 - fullpath.erase(0, posFirstColon + sepVol.length()); - } + if ( IsUNCPath(fullpath) ) + { + // 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 */ diff --git a/tests/filename/filenametest.cpp b/tests/filename/filenametest.cpp index 75035cf552..46442e2507 100644 --- a/tests/filename/filenametest.cpp +++ b/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" ); } From e8629db48ab86c4c0cd76cd0323f2fa638626041 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 15 Sep 2021 02:06:30 +0100 Subject: [PATCH 6/7] Add another test case for wxPATH_NORM_DOTS This was reported to not work but does work now. See #15012. --- tests/filename/filenametest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/filename/filenametest.cpp b/tests/filename/filenametest.cpp index 46442e2507..bebda4e250 100644 --- a/tests/filename/filenametest.cpp +++ b/tests/filename/filenametest.cpp @@ -287,6 +287,7 @@ TEST_CASE("wxFileName::Normalize", "[filename]") { "a/.././b/c/../../", wxPATH_NORM_DOTS, "", wxPATH_UNIX }, { "", wxPATH_NORM_DOTS, "", wxPATH_UNIX }, { "./foo", wxPATH_NORM_DOTS, "foo", wxPATH_UNIX }, + { "foo/./bar", wxPATH_NORM_DOTS, "foo/bar", wxPATH_UNIX }, { "b/../bar", wxPATH_NORM_DOTS, "bar", wxPATH_UNIX }, { "c/../../quux", wxPATH_NORM_DOTS, "../quux", wxPATH_UNIX }, { "/c/../../quux", wxPATH_NORM_DOTS, "/quux", wxPATH_UNIX }, From ed19e5625b3042a3911a6a27d827c7b57f095647 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 15 Sep 2021 13:10:25 +0200 Subject: [PATCH 7/7] Fix wxFileName unit test under Unix after recent changes Don't use GetVolumeSeparator() to combine UNC or GUID volumes with paths, this doesn't work because this kind of paths doesn't contain colons at all. Update the documentation to mention this. --- interface/wx/filename.h | 8 ++++++++ tests/filename/filenametest.cpp | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/interface/wx/filename.h b/interface/wx/filename.h index d8ffcb16e7..a24ee7873d 100644 --- a/interface/wx/filename.h +++ b/interface/wx/filename.h @@ -877,6 +877,14 @@ public: /** Returns the string separating the volume from the path for this format. + + Note that for @c wxPATH_DOS paths this string can only be used for + single-character volumes representing the drive letters, but not with + the UNC or GUID volumes (see their description in GetVolume() + documentation). For this reason, its use should be avoided, prefer + using wxFileName constructor and Assign() overload taking the volume + and the path as separate arguments to combining the volume and the path + into a single string using the volume separator between them. */ static wxString GetVolumeSeparator(wxPathFormat format = wxPATH_NATIVE); diff --git a/tests/filename/filenametest.cpp b/tests/filename/filenametest.cpp index bebda4e250..c38c9fa442 100644 --- a/tests/filename/filenametest.cpp +++ b/tests/filename/filenametest.cpp @@ -169,7 +169,9 @@ TEST_CASE("wxFileName::Construction", "[filename]") // if the test is run from root directory or its direct subdirectory CHECK( fn.Normalize(wxPATH_NORM_ALL, "/foo/bar/baz", fni.format) ); - if ( *fni.volume && *fni.path ) + // restrict this check to drive letter volumes as UNC and GUID volumes + // can't be combined with the path using the volume separator + if ( strlen(fni.volume) == 1 && *fni.path ) { // check that specifying the volume separately or as part of the // path doesn't make any difference