Correct two bugs in wxMBConv::FromWChar() with non NUL-terminated strings.

The variable "lenChunk" was incorrectly used as the length of the wide string
chunk which could result in wrong output.

Worse, the output buffer could be overflown for the final chunk because it
didn't have to have enough space for the trailing NUL(s) in it.

Fix both bugs and added unit tests for them.

Based on patch by Kuang-che Wu.

Closes #11486.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@62793 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

src/common/strconv.cpp

@@ -323,16 +323,22 @@ wxMBConv::FromWChar(char *dst, size_t dstLen,
     const size_t lenNul = GetMBNulLen();
     for ( const wchar_t * const srcEnd = src + srcLen;
           src < srcEnd;
-          src += wxWcslen(src) + 1 /* skip L'\0' too */ )
+          src++ /* skip L'\0' too */ )
     {
         // try to convert the current chunk
         size_t lenChunk = WC2MB(NULL, src, 0);
-
         if ( lenChunk == wxCONV_FAILED )
             return wxCONV_FAILED;
 
         dstWritten += lenChunk;
-        if ( src + lenChunk < srcEnd || isNulTerminated )
+
+        const wchar_t * const
+            chunkEnd = isNulTerminated ? srcEnd - 1 : src + wxWcslen(src);
+
+        // our return value accounts for the trailing NUL(s), unlike that of
+        // WC2MB(), however don't do it for the last NUL we artificially added
+        // ourselves above
+        if ( chunkEnd < srcEnd )
             dstWritten += lenNul;
 
         if ( dst )
@@ -340,13 +346,42 @@ wxMBConv::FromWChar(char *dst, size_t dstLen,
             if ( dstWritten > dstLen )
                 return wxCONV_FAILED;
 
-            if ( WC2MB(dst, src, lenChunk + lenNul) == wxCONV_FAILED )
+            // if we know that there is enough space in the destination buffer
+            // (because we accounted for lenNul in dstWritten above), we can
+            // convert directly in place -- but otherwise we need another
+            // temporary buffer to ensure that we don't overwrite the output
+            wxCharBuffer dstBuf;
+            char *dstTmp;
+            if ( chunkEnd == srcEnd )
+            {
+                dstBuf = wxCharBuffer(lenChunk + lenNul - 1);
+                dstTmp = dstBuf.data();
+            }
+            else
+            {
+                dstTmp = dst;
+            }
+
+            if ( WC2MB(dstTmp, src, lenChunk + lenNul) == wxCONV_FAILED )
                 return wxCONV_FAILED;
 
+            if ( dstTmp != dst )
+            {
+                // copy everything up to but excluding the terminating NUL(s)
+                // into the real output buffer
+                memcpy(dst, dstTmp, lenChunk);
+
+                // micro-optimization: if dstTmp != dst it means that chunkEnd
+                // == srcEnd and so we're done, no need to update anything below
+                break;
+            }
+
             dst += lenChunk;
-            if ( src + lenChunk < srcEnd || isNulTerminated )
+            if ( chunkEnd < srcEnd )
                 dst += lenNul;
         }
+
+        src = chunkEnd;
     }
 
     return dstWritten;

tests/mbconv/mbconvtest.cpp

@@ -81,6 +81,7 @@ private:
         CPPUNIT_TEST( Latin1Tests );
         CPPUNIT_TEST( FontmapTests );
         CPPUNIT_TEST( BufSize );
+        CPPUNIT_TEST( FromWCharTests );
 #ifdef HAVE_WCHAR_H
         CPPUNIT_TEST( UTF8_41 );
         CPPUNIT_TEST( UTF8_7f );
@@ -115,6 +116,7 @@ private:
     void LibcTests();
     void FontmapTests();
     void BufSize();
+    void FromWCharTests();
     void IconvTests();
     void Latin1Tests();
 
@@ -863,6 +865,70 @@ void MBConvTestCase::BufSize()
     CPPUNIT_ASSERT_EQUAL( '?', buf[lenMB + 2] );
 }
 
+void MBConvTestCase::FromWCharTests()
+{
+    wxCSConv conv950("CP950");
+    char mbuf[10];
+    // U+4e00 is 2 bytes (0xa4 0x40) in cp950
+    wchar_t wbuf[] = { 0x4e00, 0, 0x4e00, 0 };
+
+    // test simple ASCII text
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, conv950.FromWChar(mbuf, 0, L"a", 1));
+    CPPUNIT_ASSERT_EQUAL( '!', mbuf[0]);
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( 1, conv950.FromWChar(mbuf, 1, L"a", 1));
+    CPPUNIT_ASSERT_EQUAL( 'a', mbuf[0]);
+    CPPUNIT_ASSERT_EQUAL( '!', mbuf[1]);
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, conv950.FromWChar(mbuf, 1, L"a", 2));
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( 2, conv950.FromWChar(mbuf, 2, L"a", 2));
+    CPPUNIT_ASSERT_EQUAL( 'a', mbuf[0]);
+    CPPUNIT_ASSERT_EQUAL( '\0', mbuf[1]);
+    CPPUNIT_ASSERT_EQUAL( '!', mbuf[2]);
+
+    // test non-ASCII text, 1 wchar -> 2 char
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, conv950.FromWChar(mbuf, 0, wbuf, 1));
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, conv950.FromWChar(mbuf, 1, wbuf, 1));
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( 2, conv950.FromWChar(mbuf, 2, wbuf, 1));
+    CPPUNIT_ASSERT_EQUAL( '!', mbuf[2]);
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, conv950.FromWChar(mbuf, 2, wbuf, 2));
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( 3, conv950.FromWChar(mbuf, 3, wbuf, 2));
+    CPPUNIT_ASSERT_EQUAL( '\0', mbuf[2]);
+    CPPUNIT_ASSERT_EQUAL( '!', mbuf[3]);
+
+    // test text with embedded NUL-character and srcLen specified
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, conv950.FromWChar(mbuf, 3, wbuf, 3));
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, conv950.FromWChar(mbuf, 4, wbuf, 3));
+    CPPUNIT_ASSERT_EQUAL( 5, conv950.FromWChar(mbuf, 5, wbuf, 3));
+    CPPUNIT_ASSERT_EQUAL( '\0', mbuf[2]);
+    CPPUNIT_ASSERT_EQUAL( '!', mbuf[5]);
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, conv950.FromWChar(mbuf, 5, wbuf, 4));
+
+    memset(mbuf, '!', sizeof(mbuf));
+    CPPUNIT_ASSERT_EQUAL( 6, conv950.FromWChar(mbuf, 6, wbuf, 4));
+    CPPUNIT_ASSERT_EQUAL( '\0', mbuf[2]);
+    CPPUNIT_ASSERT_EQUAL( '\0', mbuf[5]);
+    CPPUNIT_ASSERT_EQUAL( '!', mbuf[6]);
+}
 
 WXDLLIMPEXP_BASE wxMBConv* new_wxMBConv_iconv( const char* name );