include/rtl/stringutils.hxx | 11 +- include/rtl/ustring.hxx | 2 sal/CppunitTest_sal_rtl.mk | 1 sal/qa/rtl/strings/nonconstarray.cxx | 94 ++++++++++++++++++++ sal/qa/rtl/strings/test_oustring_stringliterals.cxx | 2 sw/source/filter/ww8/ww8par.cxx | 8 + 6 files changed, 112 insertions(+), 6 deletions(-)
New commits: commit 9abaa6492899b3a8e467b08ec8a084ed3db7f518 Author: Stephan Bergmann <[email protected]> AuthorDate: Fri Sep 4 20:26:58 2020 +0200 Commit: Stephan Bergmann <[email protected]> CommitDate: Sat Sep 5 12:29:10 2020 +0200 Make OUString(char16_t const[N]) ctor check for embedded NULs ...and fix the detected fallout. That ctor only started to get used recently with a1570b6052ae9c9349282027c9007b071589bce6 "Make the OUString ConstCharArrayDetector::TypeUtf16 overloads are actually used", but it turns out that that also gave rise to that ctor being picked in error. To better guard against such erroneous uses, make that ctor assert that the given array does not contain embedded NUL characters, see the new sal/qa/rtl/strings/nonconstarray.cxx tests. The one place where that assert would fire during `make check` is fixed now in SwWW8ImplReader::ImportDopTypography. That assert would also fire for tow OUStringLiteral-related tests in the recently added test::oustring::StringLiterals::checkEmbeddedNul, so drop those for how. They cna presumably be added back (with reversed logic values) when OUStringLiteral is changed similarly to how OStringLiteral was changed in 4b9e440c51be3e40326bc90c33ae69885bfb51e4 "Turn OStringLiteral into a consteval'ed, static-refcound rtl_String". Change-Id: I6056244003a20f77ba0d953538d25edcbd562211 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102063 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <[email protected]> diff --git a/include/rtl/stringutils.hxx b/include/rtl/stringutils.hxx index 2a6f841ecaed..c7abe5e60df6 100644 --- a/include/rtl/stringutils.hxx +++ b/include/rtl/stringutils.hxx @@ -256,8 +256,14 @@ struct ConstCharArrayDetector<sal_Unicode const [N], T> { using TypeUtf16 = T; static constexpr bool const ok = true; static constexpr std::size_t const length = N - 1; - static constexpr bool isValid(sal_Unicode const (& literal)[N]) - { return literal[N - 1] == '\0'; } + static constexpr bool isValid(sal_Unicode const (& literal)[N]) { + for (std::size_t i = 0; i != N - 1; ++i) { + if (literal[i] == '\0') { + return false; + } + } + return literal[N - 1] == '\0'; + } static constexpr sal_Unicode const * toPointer( sal_Unicode const (& literal)[N]) { return literal; } @@ -269,6 +275,7 @@ template<typename T> struct ConstCharArrayDetector< using TypeUtf16 = T; static constexpr bool const ok = true; static constexpr std::size_t const length = 1; + static constexpr bool isValid(OUStringChar) { return true; } static constexpr sal_Unicode const * toPointer( OUStringChar_ const & literal) { return &literal.c; } diff --git a/include/rtl/ustring.hxx b/include/rtl/ustring.hxx index d53eab6c3038..cba3c47f5155 100644 --- a/include/rtl/ustring.hxx +++ b/include/rtl/ustring.hxx @@ -301,6 +301,8 @@ public: = libreoffice_internal::Dummy()): pData(nullptr) { + assert( + libreoffice_internal::ConstCharArrayDetector<T>::isValid(literal)); if (libreoffice_internal::ConstCharArrayDetector<T>::length == 0) { rtl_uString_new(&pData); } else { diff --git a/sal/CppunitTest_sal_rtl.mk b/sal/CppunitTest_sal_rtl.mk index c2eaa72daa26..f559202c54d5 100644 --- a/sal/CppunitTest_sal_rtl.mk +++ b/sal/CppunitTest_sal_rtl.mk @@ -29,6 +29,7 @@ $(eval $(call gb_CppunitTest_add_exception_objects,sal_rtl,\ sal/qa/rtl/process/rtl_Process \ sal/qa/rtl/random/rtl_random \ sal/qa/rtl/ref/rtl_ref \ + sal/qa/rtl/strings/nonconstarray \ sal/qa/rtl/strings/test_strings_replace \ sal/qa/rtl/strings/test_ostring \ sal/qa/rtl/strings/test_ostring_concat \ diff --git a/sal/qa/rtl/strings/nonconstarray.cxx b/sal/qa/rtl/strings/nonconstarray.cxx new file mode 100644 index 000000000000..4b66e4e311c3 --- /dev/null +++ b/sal/qa/rtl/strings/nonconstarray.cxx @@ -0,0 +1,94 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <sal/config.h> + +#include <cppunit/TestAssert.h> +#include <cppunit/TestFixture.h> +#include <cppunit/extensions/HelperMacros.h> +#include <rtl/string.hxx> +#include <rtl/ustring.hxx> + +namespace +{ +class Test : public CppUnit::TestFixture +{ +private: + void testOString() + { + struct S + { + char a[4]; + }; + S s{ "x\0y" }; + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), OString(s.a).getLength()); + // Ideally, the below would work the same as the above. However, the const reference makes + // the ConstCharArrayDetector instead of the NonConstCharArrayDetector kick in, so that the + // call to OString(r.a) would fire the ConstCharArrayDetector<T>::isValid assert (and in + // NDEBUG builds the CPPUNIT_ASSERT_EQUAL would fail with 3 != 1): + if ((false)) + { + S const& r = s; + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), OString(r.a).getLength()); + } + } + + void testOUStringChar() + { + struct S + { + char a[4]; + }; + S s{ "x\0y" }; + // This would fail to compile, as there is no OUString ctor taking a + // NonConstCharArrayDetector char array: +#if 0 + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), OUString(s.a).getLength()); +#endif + // Ideally, the below would fail to compile the same as the above. However, the const + // reference makes the ConstCharArrayDetector instead of the NonConstCharArrayDetector kick + // in, so that the call to OUString(r.a) would fire the ConstCharArrayDetector<T>::isValid + // assert (and in NDEBUG builds the CPPUNIT_ASSERT_EQUAL would fail with 3 != 1): + if ((false)) + { + S const& r = s; + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), OUString(r.a).getLength()); + } + } + + void testOUStringChar16t() + { + struct S + { + char16_t a[4]; + }; + S s{ u"x\0y" }; + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), OUString(s.a).getLength()); + // Ideally, the below would work the same as the above. However, the const reference makes + // the ConstCharArrayDetector instead of the NonConstCharArrayDetector kick in, so that the + // call to OUString(r.a) would fire the ConstCharArrayDetector<T>::isValid assert (and in + // NDEBUG builds the CPPUNIT_ASSERT_EQUAL would fail with 3 != 1):: + if ((false)) + { + S const& r = s; + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), OUString(r.a).getLength()); + } + } + + CPPUNIT_TEST_SUITE(Test); + CPPUNIT_TEST(testOString); + CPPUNIT_TEST(testOUStringChar); + CPPUNIT_TEST(testOUStringChar16t); + CPPUNIT_TEST_SUITE_END(); +}; + +CPPUNIT_TEST_SUITE_REGISTRATION(Test); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/sal/qa/rtl/strings/test_oustring_stringliterals.cxx b/sal/qa/rtl/strings/test_oustring_stringliterals.cxx index a1c79f8cc9e5..26fe12b5cf96 100644 --- a/sal/qa/rtl/strings/test_oustring_stringliterals.cxx +++ b/sal/qa/rtl/strings/test_oustring_stringliterals.cxx @@ -400,8 +400,6 @@ void test::oustring::StringLiterals::checkEmbeddedNul() { CPPUNIT_ASSERT(s.startsWith(u"foo\0hidden")); CPPUNIT_ASSERT(!s.startsWith(u"foo\0hidden"s)); CPPUNIT_ASSERT(!s.startsWith(u"foo\0hidden"sv)); - CPPUNIT_ASSERT(!s.startsWith(rtlunittest::OUStringLiteral(a))); - CPPUNIT_ASSERT(!s.startsWith(rtlunittest::OUStringLiteral(u"foo\0hidden"))); } } // namespace diff --git a/sw/source/filter/ww8/ww8par.cxx b/sw/source/filter/ww8/ww8par.cxx index 74eb087675c5..c77b0eef7b13 100644 --- a/sw/source/filter/ww8/ww8par.cxx +++ b/sw/source/filter/ww8/ww8par.cxx @@ -1967,8 +1967,12 @@ void SwWW8ImplReader::ImportDopTypography(const WW8DopTypography &rTypo) { case 2: // custom { - i18n::ForbiddenCharacters aForbidden(rTypo.m_rgxchFPunct, - rTypo.m_rgxchLPunct); + i18n::ForbiddenCharacters aForbidden(+rTypo.m_rgxchFPunct, + +rTypo.m_rgxchLPunct); + // unary + makes sure not to accidentally call the + // OUString(ConstCharArrayDetector<...>::TypeUtf16) ctor that takes the full + // m_rgxchFPunct, m_rgxchLPunct arrays with embedded NULs, instead of just the + // prefix leading up to the first NUL m_rDoc.getIDocumentSettingAccess().setForbiddenCharacters(rTypo.GetConvertedLang(), aForbidden); // Obviously cannot set the standard level 1 for japanese, so _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
