Skip to content

Commit a4ab6f1

Browse files
shoumikhinfacebook-github-bot
authored andcommitted
Fix message truncating logic to respect UTF8 encoding. (#14394)
Summary: . Reviewed By: metascroy Differential Revision: D82681904
1 parent 90ee347 commit a4ab6f1

File tree

4 files changed

+95
-15
lines changed

4 files changed

+95
-15
lines changed

runtime/platform/log.cpp

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,42 @@ static_assert(
5959
kLevelToPal[size_t(LogLevel::Fatal)] == et_pal_log_level_t::kFatal,
6060
"");
6161

62+
#if ET_LOG_ENABLED
63+
/**
64+
* Returns the length of the longest valid UTF-8 prefix in a byte buffer.
65+
*/
66+
static inline size_t get_valid_utf8_prefix_length(
67+
const char* bytes,
68+
size_t length) {
69+
if (!bytes || length == 0) {
70+
return 0;
71+
}
72+
const auto* data = reinterpret_cast<const unsigned char*>(bytes);
73+
auto index = size_t{0};
74+
auto last_valid_length = size_t{0};
75+
while (index < length) {
76+
const auto lead_byte = data[index];
77+
const size_t sequence_length = (lead_byte < 0x80) ? 1
78+
: ((lead_byte & 0xE0) == 0xC0) ? 2
79+
: ((lead_byte & 0xF0) == 0xE0) ? 3
80+
: ((lead_byte & 0xF8) == 0xF0) ? 4
81+
: 0;
82+
if (!sequence_length || index + sequence_length > length) {
83+
return last_valid_length;
84+
}
85+
for (size_t continuation_index = 1; continuation_index < sequence_length;
86+
++continuation_index) {
87+
if ((data[index + continuation_index] & 0xC0) != 0x80) {
88+
return last_valid_length;
89+
}
90+
}
91+
index += sequence_length;
92+
last_valid_length = index;
93+
}
94+
return last_valid_length;
95+
}
96+
#endif // ET_LOG_ENABLED
97+
6298
/**
6399
* Log a string message.
64100
*
@@ -84,20 +120,24 @@ void vlogf(
84120

85121
// Maximum length of a log message.
86122
static constexpr size_t kMaxLogMessageLength = 256;
87-
char buf[kMaxLogMessageLength];
88-
size_t len = vsnprintf(buf, kMaxLogMessageLength, format, args);
89-
if (len >= kMaxLogMessageLength - 1) {
90-
buf[kMaxLogMessageLength - 2] = '$';
91-
len = kMaxLogMessageLength - 1;
92-
}
93-
buf[kMaxLogMessageLength - 1] = 0;
123+
char buffer[kMaxLogMessageLength];
124+
125+
const auto write_count =
126+
vsnprintf(buffer, kMaxLogMessageLength, format, args);
127+
const size_t used_length = (write_count < 0)
128+
? 0
129+
: (write_count >= static_cast<int>(kMaxLogMessageLength)
130+
? kMaxLogMessageLength - 1
131+
: static_cast<size_t>(write_count));
132+
const auto valid_length = get_valid_utf8_prefix_length(buffer, used_length);
133+
buffer[valid_length] = '\0';
94134

95-
et_pal_log_level_t pal_level = (level < LogLevel::NumLevels)
135+
const auto pal_level = (level < LogLevel::NumLevels)
96136
? kLevelToPal[size_t(level)]
97137
: et_pal_log_level_t::kUnknown;
98138

99139
pal_emit_log_message(
100-
timestamp, pal_level, filename, function, line, buf, len);
140+
timestamp, pal_level, filename, function, line, buffer, valid_length);
101141

102142
#endif // ET_LOG_ENABLED
103143
}

runtime/platform/test/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ et_cxx_test(
3333
#
3434
# et_cxx_test(platform_death_test SOURCES executor_pal_death_test.cpp)
3535

36-
et_cxx_test(logging_test SOURCES logging_test.cpp)
36+
et_cxx_test(logging_test SOURCES logging_test.cpp stub_platform.cpp)
3737

3838
# TODO: Re-enable this test on OSS
3939
#

runtime/platform/test/logging_test.cpp

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,63 @@
1010

1111
#include <executorch/runtime/platform/log.h>
1212
#include <executorch/runtime/platform/runtime.h>
13+
#include <executorch/runtime/platform/test/pal_spy.h>
14+
#include <executorch/runtime/platform/test/stub_platform.h>
1315

1416
using namespace executorch::runtime;
1517

1618
class LoggingTest : public ::testing::Test {
17-
public:
18-
static void SetUpTestSuite() {
19-
// Initialize runtime.
20-
runtime_init();
21-
}
2219
};
2320

2421
TEST_F(LoggingTest, LogLevels) {
22+
PalSpy spy;
23+
InterceptWith iw(spy);
24+
2525
ET_LOG(Debug, "Debug log.");
26+
EXPECT_EQ(spy.last_log_message_args.message, "Debug log.");
27+
2628
ET_LOG(Info, "Info log.");
29+
EXPECT_EQ(spy.last_log_message_args.message, "Info log.");
30+
2731
ET_LOG(Error, "Error log.");
32+
EXPECT_EQ(spy.last_log_message_args.message, "Error log.");
33+
2834
ET_LOG(Fatal, "Fatal log.");
35+
EXPECT_EQ(spy.last_log_message_args.message, "Fatal log.");
2936
}
3037

3138
TEST_F(LoggingTest, LogFormatting) {
39+
PalSpy spy;
40+
InterceptWith iw(spy);
41+
3242
ET_LOG(Info, "Sample log with integer: %u", 100);
43+
EXPECT_EQ(spy.last_log_message_args.message, "Sample log with integer: 100");
44+
}
45+
46+
TEST_F(LoggingTest, Utf8Truncation) {
47+
PalSpy spy;
48+
InterceptWith iw(spy);
49+
50+
{
51+
// 253 'A' + first 2 bytes of "€" (0xE2 0x82 0xAC) -> mid-codepoint cut at 255
52+
std::string prefix(253, 'A');
53+
const char euro[] = "\xE2\x82\xAC"; // 3-byte UTF-8
54+
std::string suffix = "_SHOULD_BE_CUT";
55+
56+
ET_LOG(Info, "%s%s%s", prefix.c_str(), euro, suffix.c_str());
57+
58+
EXPECT_EQ(spy.last_log_message_args.message, prefix);
59+
EXPECT_EQ(spy.last_log_message_args.length, prefix.size());
60+
}
61+
{
62+
// 252 'B' + first 3 bytes of "👍" (0xF0 0x9F 0x91 0x8D) -> mid-codepoint cut
63+
std::string prefix(252, 'B');
64+
const char thumbs_up[] = "\xF0\x9F\x91\x8D"; // 4-byte UTF-8
65+
std::string suffix = "_SHOULD_BE_CUT";
66+
67+
ET_LOG(Info, "%s%s%s", prefix.c_str(), thumbs_up, suffix.c_str());
68+
69+
EXPECT_EQ(spy.last_log_message_args.message, prefix);
70+
EXPECT_EQ(spy.last_log_message_args.length, prefix.size());
71+
}
3372
}

runtime/platform/test/targets.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ def define_common_targets():
8484
"logging_test.cpp",
8585
],
8686
deps = [
87+
":stub_platform",
8788
"//executorch/runtime/platform:platform",
8889
],
8990
compiler_flags = [

0 commit comments

Comments
 (0)