wwbmmm commented on code in PR #3576:
URL: https://github.com/apache/brpc/pull/3576#discussion_r4151750930
##########
src/mcpack2pb/parser.cpp:
##########
@@ -299,24 +306,24 @@ int64_t UnparsedValue::as_int64(const char* var) {
if (value <= (uint64_t)std::numeric_limits<int64_t>::max()) {
return (int64_t)value;
}
- CHECK(false) << "uint64=" << value << " to " << var << " overflows";
+ LOG(ERROR) << "uint64=" << value << " to " << var << " overflows";
_stream->set_bad();
return std::numeric_limits<int64_t>::max();
}
case PRIMITIVE_FIELD_BOOL:
return _stream->cut_packed_pod<bool>();
case PRIMITIVE_FIELD_FLOAT:
- CHECK(false) << "Can't set float=" << _stream->cut_packed_pod<float>()
+ LOG(ERROR) << "Can't set float=" << _stream->cut_packed_pod<float>()
<< " to " << var;
_stream->set_bad();
return 0;
case PRIMITIVE_FIELD_DOUBLE:
- CHECK(false) << "Can't set double=" <<
_stream->cut_packed_pod<double>()
+ LOG(ERROR) << "Can't set double=" << _stream->cut_packed_pod<double>()
<< " to " << var;
_stream->set_bad();
Review Comment:
[replied by brpc-oncall robot] Valid point — the value-returning
`cut_packed_pod<T>()` returned an uninitialized local when `cutn()` short-read,
so truncated primitive payloads were consumed as garbage values (and logged,
which is UB) without failing the parse.
Fixed in the latest commit: `cut_packed_pod()` now zero-initializes its
result, and on a short read it returns a zero value and marks the stream bad,
so the generated code fails the parse via `stream()->good()` at all five call
sites you listed (as_int64/as_uint64/as_int32/as_uint32/as_bool, plus
as_float/as_double). The pointer version `cut_packed_pod(T*)` is unchanged
since its callers (e.g. `unbox()`, the iterators) already check the returned
size themselves. Added regression tests `Int32FieldTruncatedPayloadIsRejected`
and `FloatFieldTruncatedPayloadIsRejected`: the int32 case fails on the
previous code (a 1-byte truncated value was returned as 42 with `good() ==
true`) and passes now.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
