From 2730723380b41dd1020e19cee33adfa993e2940c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 6 Dec 2009 02:30:05 +0000 Subject: [PATCH] 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 | 45 +- tests/mbconv/mbconvtest.cpp | 66 + tests/test_vc9_test.vcproj | 4457 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 4540 insertions(+), 28 deletions(-) diff --git a/src/common/strconv.cpp b/src/common/strconv.cpp index db51e2c..599048e 100644 --- a/src/common/strconv.cpp +++ b/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; diff --git a/tests/mbconv/mbconvtest.cpp b/tests/mbconv/mbconvtest.cpp index b91fe59..e3870f4 100644 --- a/tests/mbconv/mbconvtest.cpp +++ b/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 ); diff --git a/tests/test_vc9_test.vcproj b/tests/test_vc9_test.vcproj index 8178698..61c7770 100644 --- a/tests/test_vc9_test.vcproj +++ b/tests/test_vc9_test.vcproj @@ -1,16 +1,10 @@ - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - -- 2.7.4