fix off-by-one truncation detection in VADoRawLog and DoRawLog - #2125
fix off-by-one truncation detection in VADoRawLog and DoRawLog#2125nabhan06 wants to merge 4 commits into
Conversation
| kExpectedDeathOutput); | ||
| } | ||
|
|
||
| TEST(RawLoggingDeathTest, TruncationMarkerAtExactBufferBoundary) { |
There was a problem hiding this comment.
This test passes without the other changes. Is there a bug here that's reproducible at head?
There was a problem hiding this comment.
Yeah, the bug is real at head. With the zero-length prefix hook the whole 3000-byte buffer is free, so an exactly-3000-char message makes vsnprintf return 3000 == size. The old n > *size check reads that as a clean fit, so the buffer gets 2999 'x' plus the embedded NUL and is flushed with no (message truncated) and no trailing newline, which lets the next raw log line concatenate onto it. n >= *size is what reserves room for the marker.
The reason the old test passed without the fix is that it leaned on EXPECT_DEATH_IF_SUPPORTED, which no-ops when death tests aren't run in a given config, so the log statement never actually executed. I've reworked it to not depend on death tests at all: it dups STDERR_FILENO onto a pipe, logs at ERROR, and asserts on the captured bytes. That version fails at head (no marker, no newline) and passes with the fix. Verified both directions locally.
The death test passed at head where death tests are skipped, so it did not guard the fix. Capture STDERR_FILENO through a pipe with a zero-length prefix hook and assert the marker and trailing newline directly; this fails at head and passes with the fix on any platform where raw logging is supported.
|
|
||
| // Install a prefix hook that writes nothing so the whole raw-log buffer is | ||
| // available to the message and the exact boundary is hit deterministically. | ||
| absl::raw_log_internal::RegisterLogFilterAndPrefixHook( |
There was a problem hiding this comment.
Unfortunately this call doesn't work internally (it asserts). We have internal hooks that invoke this and conflict here. Can you reproduce it without an internal hook?
There was a problem hiding this comment.
Good point, dropped the hook. The test now leaves the default prefix in place and just sizes the message to the space left after it, reconstructing "[basename : line] RAW: " from the same basename and source line the ABSL_RAW_LOG reports so the would-be length lands exactly on the buffer boundary. Confirmed it still fails at head (no marker, buffer ends in NUL instead of a newline) and passes with the fix.
2 participants
treat n == size as truncation in VADoRawLog and DoRawLog: vsnprintf writes only size-1 chars at that boundary, so an exactly-buffer-filling raw log message was losing its last byte and being emitted without the
(message truncated)marker or trailing newline, letting a following line concatenate onto it.