[Libreoffice-commits] core.git: include/rtl sal/CppunitTest_sal_rtl.mk sal/qa sw/source
Stephan Bergmann (via logerrit)
logerrit at kemper.freedesktop.org
Sat Sep 5 10:29:44 UTC 2020
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 <sbergman at redhat.com>
AuthorDate: Fri Sep 4 20:26:58 2020 +0200
Commit: Stephan Bergmann <sbergman at redhat.com>
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 <sbergman at redhat.com>
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
More information about the Libreoffice-commits
mailing list