Copilot commented on code in PR #50356:
URL: https://github.com/apache/arrow/pull/50356#discussion_r3540351400
##########
cpp/src/gandiva/precompiled/string_ops_test.cc:
##########
@@ -1608,6 +1609,55 @@ TEST(TestStringOps, TestRpadString) {
EXPECT_EQ(std::string(out_str + 5000, 2), "α");
}
+TEST(TestStringOps, TestPadMalformedUtf8NoOverread) {
+ gandiva::ExecutionContext ctx;
+ uint64_t ctx_ptr = reinterpret_cast<gdv_int64>(&ctx);
+ gdv_int32 out_len = 0;
+
+ // A 4-byte utf8 lead byte followed by non-continuation bytes and no trailing
+ // space. utf8_length_ignore_invalid() used to extend the glyph length past
+ // the end of the buffer while scanning the continuation bytes. The input is
+ // held in an exactly-sized heap buffer so any over-read trips
AddressSanitizer.
+ std::vector<char> text = {'\xF0', 'a', 'a', 'a'};
+ const auto text_len = static_cast<gdv_int32>(text.size());
+ const std::string text_str(text.begin(), text.end());
Review Comment:
This test intends to rely on an exactly-sized heap allocation so an
out-of-bounds read will reliably trip ASAN. Using std::vector<char> does not
guarantee capacity == size, so the over-read could remain within the vector’s
allocation and the test would still pass (making the regression test
ineffective). Consider allocating a raw heap buffer of the exact length (e.g.,
via std::unique_ptr<char[]> / new char[n]) for both malformed-UTF8 tests and
constructing the std::string from that buffer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]