Skip to content

Commit 35fe670

Browse files
Madhuri Londhemeta-codesync[bot]
authored andcommitted
Handle empty iobuf in Thrift checksum adapter
Summary: During S577904 investigation we suspect that crash is happening because nonPayload info is corrupted as ioBuf for nonPayload is set to null. We do not check if receiving ioBuf or nonPayload's iobuf are valid before accessing them. This diff doesn't fix the sev issue but we should not crash client process rather graceful handle empty iobuf by throwing exceptions and logging. Differential Revision: D85472448 fbshipit-source-id: 623d3c68620b6625d00b29254784db3895c8167f
1 parent 353eeb6 commit 35fe670

File tree

1 file changed

+47
-13
lines changed

1 file changed

+47
-13
lines changed

third-party/thrift/src/thrift/lib/cpp2/transport/rocket/payload/ChecksumPayloadSerializerStrategy.h

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -272,20 +272,54 @@ class ChecksumPayloadSerializerStrategy final
272272
FOLLY_ALWAYS_INLINE folly::Try<T> unpackImpl(
273273
Payload&& payload, DelegateFunc func) {
274274
if (payload.hasNonemptyMetadata()) {
275-
folly::Try<T> t = func(std::move(payload));
276-
bool compressed = isDataCompressed(&t.value().metadata);
277-
folly::IOBuf& buf = *t->payload.get();
278-
if (t.hasException() || compressed) {
279-
return t;
280-
} else if (validateChecksum(buf, t->metadata.checksum())) {
281-
return t;
282-
} else {
283-
if (FOLLY_LIKELY(t->metadata.checksum().has_value())) {
284-
validateInvalidChecksum(t->metadata.checksum().value());
275+
// Wrap the entire unpack and validation pipeline in try-catch to prevent
276+
// exceptions from escaping into noexcept contexts
277+
// (e.g., RocketClientChannel::onResponsePayload).
278+
// This catches:
279+
// 1. TProtocolException from delegate's unpack() (truncated data)
280+
// 2. TApplicationException from validateChecksum() (unsupported
281+
// algorithm)
282+
// 3. Any other exceptions during deserialization or validation
283+
try {
284+
folly::Try<T> t = func(std::move(payload));
285+
bool compressed = isDataCompressed(&t.value().metadata);
286+
folly::IOBuf& buf = *t->payload.get();
287+
if (t.hasException() || compressed) {
288+
return t;
289+
} else if (validateChecksum(buf, t->metadata.checksum())) {
290+
return t;
291+
} else {
292+
if (FOLLY_LIKELY(t->metadata.checksum().has_value())) {
293+
validateInvalidChecksum(t->metadata.checksum().value());
294+
}
295+
return folly::Try<T>(
296+
folly::make_exception_wrapper<TApplicationException>(
297+
TApplicationException::CHECKSUM_MISMATCH,
298+
"Checksum mismatch"));
285299
}
286-
return folly::Try<T>(
287-
folly::make_exception_wrapper<TApplicationException>(
288-
TApplicationException::CHECKSUM_MISMATCH, "Checksum mismatch"));
300+
} catch (...) {
301+
// Catch and wrap any exceptions to prevent termination in noexcept
302+
// contexts
303+
auto ex = std::current_exception();
304+
try {
305+
std::rethrow_exception(ex);
306+
} catch (const std::exception& e) {
307+
// Log detailed information to help identify the source of malformed
308+
// responses
309+
XLOG(ERR)
310+
<< "ChecksumPayloadSerializer: Exception during unpack/validation. "
311+
<< "Exception type: " << folly::demangle(typeid(e))
312+
<< ", Message: " << e.what() << ", Payload metadata size: "
313+
<< (payload.hasNonemptyMetadata() ? payload.metadataSize() : 0)
314+
<< ", Payload data size: " << payload.dataSize();
315+
} catch (...) {
316+
XLOG(ERR)
317+
<< "ChecksumPayloadSerializer: Unknown exception during unpack/validation. "
318+
<< "Payload metadata size: "
319+
<< (payload.hasNonemptyMetadata() ? payload.metadataSize() : 0)
320+
<< ", Payload data size: " << payload.dataSize();
321+
}
322+
return folly::Try<T>(folly::exception_wrapper(ex));
289323
}
290324
} else {
291325
return func(std::move(payload));

0 commit comments

Comments
 (0)