include/rtl/strbuf.hxx | 3
include/rtl/ustrbuf.hxx | 3
sal/CppunitTest_sal_rtl.mk | 1
sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx | 8 ++
sal/qa/rtl/strings/test_ostringbuffer.cxx | 56 +++++++++++++++
5 files changed, 69 insertions(+), 2 deletions(-)
New commits:
commit ca832495ac83117bed6c334f3cc72d6a84d66a6c
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Thu Mar 31 09:30:24 2022 +0200
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Thu Mar 31 18:52:48 2022 +0200
A string_view is not necessarily NUL-terminated
Regression introduced into LIBO_INTERNAL_ONLY code with
1da69081732c8a429840edaaf10cfb789ea68df8 "add string_view variants of
methods to
O[U]StringBuffer".
And add some tests. Assigning an OStringChar to an OStringBuffer also uses
the
std::string_view assignment operator, so also test that with
testOStringChar,
even though there it is relatively likely that the OStringChar temporary is
followed by null bytes, which could make the test happen to erroneously
succeed.
But at least tools like ASan or Valgrind could catch that. On the other
hand,
assigning an OUStringChar to an OUStringBuffer does not use the
std::u16string_view assignment operator (and rather uses a
ConstCharArrayDetector-based one, which was similarly broken and has been
fixed
in b66387574ef9c83cbfff622468496b6f0ac4d571 "Fix -Werror=array-bounds"), so
there's no test for that here.
Change-Id: I7cf10ee5ce0e4280a91d116cd82d4871a0f44af6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132363
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
diff --git a/include/rtl/strbuf.hxx b/include/rtl/strbuf.hxx
index 52399b6da8b4..5c8080466365 100644
--- a/include/rtl/strbuf.hxx
+++ b/include/rtl/strbuf.hxx
@@ -277,7 +277,8 @@ public:
if (n >= nCapacity) {
ensureCapacity(n + 16); //TODO: check for overflow
}
- std::memcpy(pData->buffer, string.data(), n + 1);
+ std::memcpy(pData->buffer, string.data(), n);
+ pData->buffer[n] = '\0';
pData->length = n;
return *this;
}
diff --git a/include/rtl/ustrbuf.hxx b/include/rtl/ustrbuf.hxx
index fef71f98b329..fa8fa511071a 100644
--- a/include/rtl/ustrbuf.hxx
+++ b/include/rtl/ustrbuf.hxx
@@ -294,7 +294,8 @@ public:
}
std::memcpy(
pData->buffer, string.data(),
- (n + 1) * sizeof (sal_Unicode));
+ n * sizeof (sal_Unicode));
+ pData->buffer[n] = '\0';
pData->length = n;
return *this;
}
diff --git a/sal/CppunitTest_sal_rtl.mk b/sal/CppunitTest_sal_rtl.mk
index d5f11e49b6d1..2568d8dde70c 100644
--- a/sal/CppunitTest_sal_rtl.mk
+++ b/sal/CppunitTest_sal_rtl.mk
@@ -37,6 +37,7 @@ $(eval $(call gb_CppunitTest_add_exception_objects,sal_rtl,\
sal/qa/rtl/strings/test_ostring \
sal/qa/rtl/strings/test_ostring_concat \
sal/qa/rtl/strings/test_ostring_stringliterals \
+ sal/qa/rtl/strings/test_ostringbuffer \
sal/qa/rtl/strings/test_oustring_compare \
sal/qa/rtl/strings/test_oustring_concat \
sal/qa/rtl/strings/test_oustring_convert \
diff --git a/sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx
b/sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx
index da5a4634e94b..3c3f436ee5c0 100644
--- a/sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx
+++ b/sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx
@@ -9,6 +9,8 @@
#include <sal/config.h>
+#include <string_view>
+
#include <cppunit/TestFixture.h>
#include <cppunit/TestAssert.h>
#include <cppunit/extensions/HelperMacros.h>
@@ -71,6 +73,12 @@ private:
CPPUNIT_ASSERT_EQUAL(sal_Int32(1), b4.getLength());
CPPUNIT_ASSERT_EQUAL(u'a', b4.getStr()[0]);
CPPUNIT_ASSERT_EQUAL(u'\0', b4.getStr()[1]);
+ b4 = std::u16string_view(u"abc").substr(
+ 0, 2); // avoid the string_view accidentally being NUL-terminated
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(2), b4.getLength());
+ CPPUNIT_ASSERT_EQUAL(u'a', b4.getStr()[0]);
+ CPPUNIT_ASSERT_EQUAL(u'b', b4.getStr()[1]);
+ CPPUNIT_ASSERT_EQUAL(u'\0', b4.getStr()[2]);
}
CPPUNIT_TEST_SUITE(Test);
diff --git a/sal/qa/rtl/strings/test_ostringbuffer.cxx
b/sal/qa/rtl/strings/test_ostringbuffer.cxx
new file mode 100644
index 000000000000..292e24310b7b
--- /dev/null
+++ b/sal/qa/rtl/strings/test_ostringbuffer.cxx
@@ -0,0 +1,56 @@
+/* -*- 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 <string_view>
+
+#include <cppunit/TestFixture.h>
+#include <cppunit/TestAssert.h>
+#include <cppunit/extensions/HelperMacros.h>
+
+#include <rtl/strbuf.hxx>
+#include <rtl/stringutils.hxx>
+#include <sal/types.h>
+
+namespace
+{
+class Test : public CppUnit::TestFixture
+{
+private:
+ void testStringView()
+ {
+ OStringBuffer b("foobar");
+ b = std::string_view("abc").substr(
+ 0, 2); // avoid the string_view accidentally being NUL-terminated
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(2), b.getLength());
+ CPPUNIT_ASSERT_EQUAL('a', b.getStr()[0]);
+ CPPUNIT_ASSERT_EQUAL('b', b.getStr()[1]);
+ CPPUNIT_ASSERT_EQUAL('\0', b.getStr()[2]);
+ }
+
+ void testOStringChar()
+ {
+ OStringBuffer b("foobar");
+ b = OStringChar('a');
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(1), b.getLength());
+ CPPUNIT_ASSERT_EQUAL('a', b.getStr()[0]);
+ CPPUNIT_ASSERT_EQUAL('\0', b.getStr()[1]);
+ }
+
+ CPPUNIT_TEST_SUITE(Test);
+ CPPUNIT_TEST(testStringView);
+ CPPUNIT_TEST(testOStringChar);
+ CPPUNIT_TEST_SUITE_END();
+};
+}
+
+CPPUNIT_TEST_SUITE_REGISTRATION(Test);
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
