Merge fixes for Unicode support in wxFileSystem::FindFirst()

Closes #11404.

docs/changes.txt

@@ -63,6 +63,7 @@ All:
 
 - Add UTF-8 and ZIP 64 support to wxZip{Input,Output}Stream (Tobias Taschner).
 - Upgrade libpng to 1.6.21 fixing several security bugs (Paul Kulchenko).
+- Fix handling of Unicode file names in wxFileSystem::FindFirst().
 - Add wxStandardPaths::GetUserDir() (Tobias Taschner).
 - Allow calling wxItemContainer::Add() and similar with std::vector<> argument.
 - Add "%z" support to printf()-like functions like wxString::Format() (RIVDSL).

include/wx/uri.h

@@ -137,11 +137,6 @@ protected:
     static bool ParseIPv6address(const char*& uri);
     static bool ParseIPvFuture(const char*& uri);
 
-    // should be called with i pointing to '%', returns the encoded character
-    // following it or -1 if invalid and advances i past it (so that it points
-    // to the last character consumed on return)
-    static int DecodeEscape(wxString::const_iterator& i);
-
     // append next character pointer to by p to the string in an escaped form
     // and advance p past it
     //

src/common/uri.cpp

@@ -90,9 +90,9 @@ void wxURI::Clear()
 /* static */
 int wxURI::CharToHex(char c)
 {
-    if ((c >= 'A') && (c <= 'Z'))
+    if ((c >= 'A') && (c <= 'F'))
         return c - 'A' + 10;
-    if ((c >= 'a') && (c <= 'z'))
+    if ((c >= 'a') && (c <= 'f'))
         return c - 'a' + 10;
     if ((c >= '0') && (c <= '9'))
         return c - '0';
@@ -100,38 +100,32 @@ int wxURI::CharToHex(char c)
     return -1;
 }
 
-int wxURI::DecodeEscape(wxString::const_iterator& i)
-{
-    int hi = CharToHex(*++i);
-    if ( hi == -1 )
-        return -1;
-
-    int lo = CharToHex(*++i);
-    if ( lo == -1 )
-        return -1;
-
-    return (hi << 4) | lo;
-}
-
 /* static */
 wxString wxURI::Unescape(const wxString& uri)
 {
+    // URIs can contain escaped 8-bit characters that have to be decoded using
+    // UTF-8 (see RFC 3986), however in our (probably broken...) case we can
+    // also end up with not escaped Unicode characters in the URI string which
+    // can't be decoded as UTF-8. So what we do here is to encode all Unicode
+    // characters as UTF-8 only to decode them back below. This is obviously
+    // inefficient but there doesn't seem to be anything else to do, other than
+    // not allowing to mix Unicode characters with escapes in the first place,
+    // but this seems to be done in a lot of places, unfortunately.
+    const wxScopedCharBuffer& uriU8(uri.utf8_str());
+    const size_t len = uriU8.length();
+
     // the unescaped version can't be longer than the original one
-    wxCharBuffer buf(uri.length());
+    wxCharBuffer buf(uriU8.length());
     char *p = buf.data();
 
-    for ( wxString::const_iterator i = uri.begin(); i != uri.end(); ++i, ++p )
+    const char* const end = uriU8.data() + len;
+    for ( const char* s = uriU8.data(); s != end; ++s, ++p )
     {
-        char c = *i;
+        char c = *s;
-        if ( c == '%' )
+        if ( c == '%' && s < end - 2 && IsHex(s[1]) && IsHex(s[2]) )
         {
-            int n = wxURI::DecodeEscape(i);
+            c = (CharToHex(s[1]) << 4) | CharToHex(s[2]);
-            if ( n == -1 )
+            s += 2;
-                return wxString();
-
-            wxASSERT_MSG( n >= 0 && n <= 0xff, "unexpected character value" );
-
-            c = static_cast<char>(n);
         }
 
         *p = c;
@@ -139,17 +133,7 @@ wxString wxURI::Unescape(const wxString& uri)
 
     *p = '\0';
 
-    // by default assume that the URI is in UTF-8, this is the most common
+    return wxString::FromUTF8(buf);
-    // practice
-    wxString s = wxString::FromUTF8(buf);
-    if ( s.empty() )
-    {
-        // if it isn't, use latin-1 as a fallback -- at least this always
-        // succeeds
-        s = wxCSConv(wxFONTENCODING_ISO8859_1).cMB2WC(buf);
-    }
-
-    return s;
 }
 
 void wxURI::AppendNextEscaped(wxString& s, const char *& p)

tests/file/filefn.cpp

@@ -20,6 +20,8 @@
 
 #include "wx/ffile.h"
 #include "wx/filefn.h"
+#include "wx/textfile.h"
+#include "wx/filesys.h"
 
 #include "testfile.h"
 
@@ -31,13 +33,50 @@ class FileFunctionsTestCase : public CppUnit::TestCase
 {
 public:
     FileFunctionsTestCase() { }
+    void setUp();
+    void tearDown();
 
 private:
     CPPUNIT_TEST_SUITE( FileFunctionsTestCase );
+        CPPUNIT_TEST( GetTempFolder );
         CPPUNIT_TEST( CopyFile );
+        CPPUNIT_TEST( CreateFile );
+        CPPUNIT_TEST( FileExists );
+        CPPUNIT_TEST( FindFile );
+        CPPUNIT_TEST( FindFileNext );
+        CPPUNIT_TEST( RemoveFile );
+        CPPUNIT_TEST( RenameFile );
+        CPPUNIT_TEST( ConcatenateFiles );
+        CPPUNIT_TEST( GetCwd );
     CPPUNIT_TEST_SUITE_END();
 
+    void GetTempFolder();
     void CopyFile();
+    void CreateFile();
+    void FileExists();
+    void FindFile();
+    void FindFileNext();
+    void RemoveFile();
+    void RenameFile();
+    void ConcatenateFiles();
+    void GetCwd();
+
+    // Helper methods
+    void DoCreateFile(const wxString& filePath);
+    void DoFileExists(const wxString& filePath);
+    void DoFindFile(const wxString& filePath);
+    void DoRemoveFile(const wxString& filePath);
+    void DoRenameFile(const wxString& oldFilePath,
+                      const wxString& newFilePath,
+                      bool overwrite,
+                      bool withNew);
+    void DoConcatFile(const wxString& filePath1,
+                      const wxString& filePath2,
+                      const wxString& destFilePath);
+
+    wxString m_fileNameASCII;
+    wxString m_fileNameNonASCII;
+    wxString m_fileNameWork;
 
     wxDECLARE_NO_COPY_CLASS(FileFunctionsTestCase);
 };
@@ -53,34 +92,340 @@ CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( FileFunctionsTestCase, "FileFunctionsTest
 // tests implementation
 // ----------------------------------------------------------------------------
 
+void FileFunctionsTestCase::setUp()
+{
+    // Initialize local data
+
+    wxFileName fn1(wxFileName::GetTempDir(), wxT("wx_file_mask.txt"));
+    m_fileNameASCII = fn1.GetFullPath();
+
+    // This file name is 'wx_file_mask.txt' in Russian.
+    wxFileName fn2(wxFileName::GetTempDir(),
+      wxT("wx_\u043C\u0430\u0441\u043A\u0430_\u0444\u0430\u0439\u043B\u0430.txt"));
+    m_fileNameNonASCII = fn2.GetFullPath();
+
+    wxFileName fn3(wxFileName::GetTempDir(), wxT("wx_test_copy"));
+    m_fileNameWork = fn3.GetFullPath();
+}
+
+void FileFunctionsTestCase::tearDown()
+{
+    // Remove all remaining temporary files
+    if ( wxFileExists(m_fileNameASCII) )
+    {
+        wxRemoveFile(m_fileNameASCII);
+    }
+    if ( wxFileExists(m_fileNameNonASCII) )
+    {
+        wxRemoveFile(m_fileNameNonASCII);
+    }
+    if ( wxFileExists(m_fileNameWork) )
+    {
+        wxRemoveFile(m_fileNameWork);
+    }
+}
+
+void FileFunctionsTestCase::GetTempFolder()
+{
+    // Verify that obtained temporary folder is not empty.
+    wxString tmpDir = wxFileName::GetTempDir();
+
+    CPPUNIT_ASSERT( !tmpDir.IsEmpty() );
+}
+
 void FileFunctionsTestCase::CopyFile()
 {
-    static const wxChar *filename1 = wxT("horse.bmp");
+    const wxString filename1(wxT("horse.bmp"));
-    static const wxChar *filename2 = wxT("test_copy");
+    const wxString& filename2 = m_fileNameWork;
-    
+
-    CPPUNIT_ASSERT( wxCopyFile(filename1, filename2) );
+    const wxString msg = wxString::Format(wxT("File 1: %s  File 2:%s"),
+                                          filename1.c_str(), filename2.c_str());
+    const char *pUnitMsg = (const char*)msg.mb_str(wxConvUTF8);
+
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxCopyFile(filename1, filename2) );
 
     // verify that the two files have the same contents!
     wxFFile f1(filename1, wxT("rb")),
             f2(filename2, wxT("rb"));
 
-    CPPUNIT_ASSERT( f1.IsOpened() && f2.IsOpened() );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f1.IsOpened() && f2.IsOpened() );
-    
+
     wxString s1, s2;
-    CPPUNIT_ASSERT( f1.ReadAll(&s1) && f2.ReadAll(&s2) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f1.ReadAll(&s1) && f2.ReadAll(&s2) );
-    CPPUNIT_ASSERT( (s1.length() == s2.length()) &&
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, (s1.length() == s2.length()) &&
                     (memcmp(s1.c_str(), s2.c_str(), s1.length()) == 0) );
 
-    CPPUNIT_ASSERT( f1.Close() && f2.Close() );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f1.Close() && f2.Close() );
-    CPPUNIT_ASSERT( wxRemoveFile(filename2) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxRemoveFile(filename2) );
 }
 
+void FileFunctionsTestCase::CreateFile()
+{
+    // Create file name containing ASCII characters only.
+    DoCreateFile(m_fileNameASCII);
+    // Create file name containing non-ASCII characters.
+    DoCreateFile(m_fileNameNonASCII);
+}
+
+void FileFunctionsTestCase::DoCreateFile(const wxString& filePath)
+{
+    const wxString msg = wxString::Format(wxT("File: %s"),
+                                          filePath.c_str());
+    const char *pUnitMsg = (const char*)msg.mb_str(wxConvUTF8);
+
+    // Create temporary file.
+    wxTextFile file;
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Create(filePath) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Close() );
+
+    wxRemoveFile(filePath);
+}
+
+void FileFunctionsTestCase::FileExists()
+{
+    CPPUNIT_ASSERT( wxFileExists(wxT("horse.png")) );
+    CPPUNIT_ASSERT( !wxFileExists(wxT("horse.___")) );
+
+    // Check file name containing ASCII characters only.
+    DoFileExists(m_fileNameASCII);
+    // Check file name containing non-ASCII characters.
+    DoFileExists(m_fileNameNonASCII);
+}
+
+void FileFunctionsTestCase::DoFileExists(const wxString& filePath)
+{
+    const wxString msg = wxString::Format(wxT("File: %s"),
+                                          filePath.c_str());
+    const char *pUnitMsg = (const char*)msg.mb_str(wxConvUTF8);
+
+    // Create temporary file.
+    wxTextFile file;
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Create(filePath) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Close() );
+
+    // Verify that file exists with 2 methods.
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Exists() );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(filePath) );
+
+    wxRemoveFile(filePath);
+}
+
+void FileFunctionsTestCase::FindFile()
+{
+    // Find file name containing ASCII characters only.
+    DoFindFile(m_fileNameASCII);
+    // Find file name containing non-ASCII characters.
+    DoFindFile(m_fileNameNonASCII);
+}
+
+void FileFunctionsTestCase::DoFindFile(const wxString& filePath)
+{
+    const wxString msg = wxString::Format(wxT("File: %s"),
+                                          filePath.c_str());
+    const char *pUnitMsg = (const char*)msg.mb_str(wxConvUTF8);
+
+    // Create temporary file.
+    wxTextFile file;
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Create(filePath) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Close() );
+
+    // Check if file can be found (method 1).
+    wxString foundFile = wxFindFirstFile(filePath, wxFILE);
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, foundFile == filePath );
+
+    // Check if file can be found (method 2).
+    wxFileSystem fs;
+    wxString furl = fs.FindFirst(filePath, wxFILE);
+    wxFileName fname = wxFileSystem::URLToFileName(furl);
+    foundFile = fname.GetFullPath();
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, foundFile == filePath );
+
+    wxRemoveFile(filePath);
+}
+
+void FileFunctionsTestCase::FindFileNext()
+{
+    // Construct file name containing ASCII characters only.
+    const wxString fileMask(wxT("horse.*"));
+
+    // Check using method 1.
+    wxString foundFile1 = wxFindFirstFile(fileMask, wxFILE);
+    wxString foundFile2 = wxFindNextFile();
+    wxFileName fn1(foundFile1);
+    wxFileName fn2(foundFile2);
+    // Full names must be different.
+    CPPUNIT_ASSERT( foundFile1 != foundFile2 );
+    // Base names must be the same.
+    CPPUNIT_ASSERT( fn1.GetName() == fn2.GetName() );
+
+    // Check using method 2.
+    wxFileSystem fs;
+    wxString furl = fs.FindFirst(fileMask, wxFILE);
+    fn1 = wxFileSystem::URLToFileName(furl);
+    foundFile1 = fn1.GetFullPath();
+    furl = fs.FindNext();
+    fn2 = wxFileSystem::URLToFileName(furl);
+    foundFile2 = fn2.GetFullPath();
+    // Full names must be different.
+    CPPUNIT_ASSERT( fn1.GetFullPath() != fn2.GetFullPath() );
+    // Base names must be the same.
+    CPPUNIT_ASSERT( fn1.GetName() == fn2.GetName() );
+}
+
+void FileFunctionsTestCase::RemoveFile()
+{
+    // Create & remove file with name containing ASCII characters only.
+    DoRemoveFile(m_fileNameASCII);
+    // Create & remove file with name containing non-ASCII characters.
+    DoRemoveFile(m_fileNameNonASCII);
+}
+
+void FileFunctionsTestCase::DoRemoveFile(const wxString& filePath)
+{
+    const wxString msg = wxString::Format(wxT("File: %s"),
+                                          filePath.c_str());
+    const char *pUnitMsg = (const char*)msg.mb_str(wxConvUTF8);
+
+    // Create temporary file.
+    wxTextFile file;
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Create(filePath) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Close() );
+
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Exists() );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxRemoveFile(filePath) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, !file.Exists() );
+}
+
+void FileFunctionsTestCase::RenameFile()
+{
+    // Verify renaming file with/without overwriting
+    // when new file already exist/don't exist.
+    DoRenameFile(m_fileNameASCII, m_fileNameNonASCII, false, false);
+    DoRenameFile(m_fileNameASCII, m_fileNameNonASCII, false, true);
+    DoRenameFile(m_fileNameASCII, m_fileNameNonASCII, true, false);
+    DoRenameFile(m_fileNameASCII, m_fileNameNonASCII, true, true);
+    DoRenameFile(m_fileNameNonASCII, m_fileNameASCII, false, false);
+    DoRenameFile(m_fileNameNonASCII, m_fileNameASCII, false, true);
+    DoRenameFile(m_fileNameNonASCII, m_fileNameASCII, true, false);
+    DoRenameFile(m_fileNameNonASCII, m_fileNameASCII, true, true);
+}
+
+void
+FileFunctionsTestCase::DoRenameFile(const wxString& oldFilePath,
+                                    const wxString& newFilePath,
+                                    bool overwrite,
+                                    bool withNew)
+{
+    const wxString msg = wxString::Format(wxT("File 1: %s  File 2: %s"),
+                                          oldFilePath.c_str(), newFilePath.c_str());
+    const char *pUnitMsg = (const char*)msg.mb_str(wxConvUTF8);
+
+    // Create temporary source file.
+    wxTextFile file;
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Create(oldFilePath) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Close() );
+
+    if ( withNew )
+    {
+        // Create destination file to test overwriting.
+        wxTextFile file2;
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file2.Create(newFilePath) );
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file2.Close() );
+
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(newFilePath) );
+    }
+    else
+    {
+        // Remove destination file
+        if ( wxFileExists(newFilePath) )
+        {
+            wxRemoveFile(newFilePath);
+        }
+
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, !wxFileExists(newFilePath) );
+    }
+
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(oldFilePath) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(oldFilePath) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(oldFilePath) );
+    bool shouldFail = !overwrite && withNew;
+    if ( shouldFail )
+    {
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, !wxRenameFile(oldFilePath, newFilePath, overwrite));
+        // Verify that file has not been renamed.
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(oldFilePath) );
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(newFilePath) );
+
+        // Cleanup.
+        wxRemoveFile(oldFilePath);
+    }
+    else
+    {
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxRenameFile(oldFilePath, newFilePath, overwrite) );
+        // Verify that file has been renamed.
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, !wxFileExists(oldFilePath) );
+        CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(newFilePath) );
+    }
+
+    // Cleanup.
+    wxRemoveFile(newFilePath);
+}
+
+void FileFunctionsTestCase::ConcatenateFiles()
+{
+    DoConcatFile(m_fileNameASCII, m_fileNameNonASCII, m_fileNameWork);
+    DoConcatFile(m_fileNameNonASCII, m_fileNameASCII, m_fileNameWork);
+}
+
+void FileFunctionsTestCase::DoConcatFile(const wxString& filePath1,
+                                         const wxString& filePath2,
+                                         const wxString& destFilePath)
+{
+    const wxString msg = wxString::Format(wxT("File 1: %s  File 2: %s  File 3: %s"),
+                                  filePath1.c_str(), filePath2.c_str(), destFilePath.c_str());
+    const char *pUnitMsg = (const char*)msg.mb_str(wxConvUTF8);
+
+    // Prepare source data
+    wxFFile f1(filePath1, wxT("wb")),
+            f2(filePath2, wxT("wb"));
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f1.IsOpened() && f2.IsOpened() );
+
+    wxString s1(wxT("1234567890"));
+    wxString s2(wxT("abcdefghij"));
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f1.Write(s1) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f2.Write(s2) );
+
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f1.Close() && f2.Close() );
+
+    // Concatenate source files
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxConcatFiles(filePath1, filePath2, destFilePath) );
+
+    // Verify content of destination file
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxFileExists(destFilePath) );
+    wxString sSrc = s1 + s2;
+    wxString s3;
+    wxFFile f3(destFilePath, wxT("rb"));
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f3.ReadAll(&s3) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, (sSrc.length() == s3.length()) &&
+                    (memcmp(sSrc.c_str(), s3.c_str(), sSrc.length()) == 0) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, f3.Close() );
+
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxRemoveFile(filePath1) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxRemoveFile(filePath2) );
+    CPPUNIT_ASSERT_MESSAGE( pUnitMsg, wxRemoveFile(destFilePath) );
+}
+
+void FileFunctionsTestCase::GetCwd()
+{
+    // Verify that obtained working directory is not empty.
+    wxString cwd = wxGetCwd();
+
+    CPPUNIT_ASSERT( !cwd.IsEmpty() );
+}
 
 /*
     TODO: other file functions to test:
 
-bool wxFileExists(const wxString& filename);
-
 bool wxDirExists(const wxString& pathName);
 
 bool wxIsAbsolutePath(const wxString& filename);
@@ -90,21 +435,10 @@ wxString wxFileNameFromPath(const wxString& path);
 
 wxString wxPathOnly(const wxString& path);
 
-wxString wxFindFirstFile(const wxString& spec, int flags = wxFILE);
-wxString wxFindNextFile();
-
 bool wxIsWild(const wxString& pattern);
 
 bool wxMatchWild(const wxString& pattern,  const wxString& text, bool dot_special = true);
 
-bool wxConcatFiles(const wxString& file1, const wxString& file2, const wxString& file3);
-
-bool wxRemoveFile(const wxString& file);
-
-bool wxRenameFile(const wxString& file1, const wxString& file2, bool overwrite = true);
-
-wxString wxGetCwd();
-
 bool wxSetWorkingDirectory(const wxString& d);
 
 bool wxMkdir(const wxString& dir, int perm = wxS_DIR_DEFAULT);

tests/uris/uris.cpp

@@ -338,6 +338,15 @@ void URITestCase::Unescaping()
                             "\xD1\x87\xD0\xB8\xD1\x81\xD0\xBB\xD0\xBE"
                           ),
                           unescaped );
+
+    escaped = L"file://\u043C\u043E\u0439%5C%d1%84%d0%b0%d0%b9%d0%bb";
+    unescaped = wxURI::Unescape(escaped);
+
+    CPPUNIT_ASSERT_EQUAL
+    (
+        L"file://\u043C\u043E\u0439\\\u0444\u0430\u0439\u043B",
+        unescaped
+    );
 #endif // wxUSE_UNICODE
 }