From 30adb1713b84f48ab24319c84f994f6396141eb3 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Tue, 5 Sep 2023 22:43:55 -0700 Subject: [PATCH] Use memmove instead of strcat strcat does not allow overlapping ranges; we didn't have a test for this but now we do. As an added bonus, this also means we only compute the length of each fragment once now. --- src/pugixml.cpp | 20 +++++++------------- tests/test_parse.cpp | 8 ++++++++ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 6690df7..b076d7f 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -270,18 +270,6 @@ PUGI_IMPL_NS_BEGIN return static_cast(end - s); #endif } - - // Append one string to another - PUGI_IMPL_FN void strconcat(char_t* dst, const char_t* src) - { - assert(dst && src); - - #ifdef PUGIXML_WCHAR_MODE - wcscat(dst, src); - #else - strcat(dst, src); - #endif - } PUGI_IMPL_NS_END // auto_ptr-like object for exception recovery @@ -3498,8 +3486,14 @@ PUGI_IMPL_NS_BEGIN { assert(merged >= cursor->first_child->prev_sibling_c->value); + // Catch up to the end of last parsed value; only needed for the first fragment. merged += strlength(merged); - strconcat(merged, parsed_pcdata); // Append PCDATA to the previous one. + + size_t length = strlength(parsed_pcdata); + + // Must use memmove instead of memcpy as this move may overlap + memmove(merged, parsed_pcdata, (length + 1) * sizeof(char_t)); + merged += length; } else { diff --git a/tests/test_parse.cpp b/tests/test_parse.cpp index b38893d..4bfd6b2 100644 --- a/tests/test_parse.cpp +++ b/tests/test_parse.cpp @@ -1350,6 +1350,14 @@ TEST(parse_merge_pcdata_append) CHECK_STRING(doc.child(STR("node")).first_child().value(), STR("hello world")); } +TEST(parse_merge_pcdata_overlap) +{ + xml_document doc; + xml_parse_result res = doc.load_string(STR("short this string is very long so long that copying it will overlap itself"), parse_merge_pcdata); + CHECK(res); + CHECK_STRING(doc.child_value(STR("node")), STR("short this string is very long so long that copying it will overlap itself")); +} + TEST(parse_encoding_detect) { char test[] = "";