iemejia commented on code in PR #3859:
URL: https://github.com/apache/avro/pull/3859#discussion_r3567528107
##########
lang/c++/impl/Generic.cc:
##########
@@ -105,11 +267,38 @@ void GenericReader::read(GenericDatum &datum, Decoder &d,
bool isResolving) {
const NodePtr &nn = v.schema()->leafAt(0);
r.resize(0);
size_t start = 0;
+ // Only when not resolving: the datum schema then matches the wire
+ // schema, so minBytesPerElement is a true lower bound. Under
+ // resolution the wire (writer) type may be smaller than the datum
+ // (reader) type, which would over-estimate and reject valid data.
+ int64_t trueMin = minBytesPerElement(nn, 0);
+ // Under resolution the on-wire (writer) element can be zero bytes
+ // even when the reader element is not (e.g. reader-only fields
filled
+ // from defaults), so the bytes check is disabled and we cannot
tell
+ // whether an element is zero-byte on the wire. Apply the tighter
+ // zero-byte cap conservatively in that case, so the up-front
resize
+ // cannot be driven past it.
+ bool zeroByte = isResolving || trueMin == 0;
Review Comment:
This is a deliberate, conservative tradeoff. Under resolution the
datum/reader element schema (`nn`) may differ from the on-wire writer element,
and a writer element can be zero-byte even when the reader element isn't (e.g.
reader-only fields filled from defaults). Since minBytesPerElement(reader)
would then over-estimate the writer's on-wire size and falsely reject valid
data, the bytes-remaining check is disabled (minBytes=0) and the zero-byte cap
is applied so a genuinely-zero-byte writer element can't drive a huge resize
from a tiny payload.
The structural cap (default Integer.MAX_VALUE - 8) still always applies; the
extra bound under resolution is the zero-byte item cap (default 10,000,000),
which is raisable via `AVRO_MAX_COLLECTION_ITEMS`. Applying the precise
per-element bytes bound under resolution would require plumbing the *writer*
element schema's minimum on-wire size into this read path (the ResolvingDecoder
doesn't expose it here), which is a larger refactor. Given a legit >10M-element
resolving array can raise the limit via the env var, keeping the conservative
cap here is the safer default; I can file a follow-up for the
precise-writer-min-bytes bound if you'd prefer.
--
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]
