Copilot commented on code in PR #3859:
URL: https://github.com/apache/avro/pull/3859#discussion_r3565068622
##########
lang/c++/impl/Generic.cc:
##########
@@ -105,7 +156,13 @@ 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 minBytes = isResolving ? 0 : minBytesPerElement(nn, 0);
for (size_t m = d.arrayStart(); m != 0; m = d.arrayNext()) {
+ ensureCollectionAvailable(d, m, minBytes);
r.resize(r.size() + m);
Review Comment:
r.resize(r.size() + m) can overflow size_t before the resize call when m is
very large, potentially wrapping to a small size and causing incorrect decoding
behavior. It also loses the opportunity to fail early with a clear error when
the block count exceeds the container’s max_size().
##########
lang/c++/impl/Stream.cc:
##########
@@ -84,6 +84,15 @@ class MemoryInputStream : public InputStream {
size_t byteCount() const final {
return cur_ * chunkSize_ + curLen_;
}
+
+ int64_t remainingBytes() const final {
+ // Total capacity across all chunks: full chunks plus the (partial)
+ // last chunk, minus what has already been consumed.
+ int64_t total = (size_ == 0)
+ ? 0
+ : static_cast<int64_t>((size_ - 1) * chunkSize_ +
available_);
+ return total - static_cast<int64_t>(cur_ * chunkSize_ + curLen_);
+ }
Review Comment:
remainingBytes() computes total/consumed using size_t arithmetic inside
static_cast<int64_t>(...) (e.g. (size_-1)*chunkSize_). On large buffers this
can overflow before the cast, yielding an incorrect (possibly negative)
remaining count and disabling the available-bytes guard upstream.
##########
lang/c++/impl/Generic.cc:
##########
@@ -119,7 +176,10 @@ void GenericReader::read(GenericDatum &datum, Decoder &d,
bool isResolving) {
const NodePtr &nn = v.schema()->leafAt(1);
r.resize(0);
size_t start = 0;
+ // Map keys are strings (>= 1 byte length prefix) plus the value.
+ int64_t minBytes = isResolving ? 0 : (1 + minBytesPerElement(nn,
0));
for (size_t m = d.mapStart(); m != 0; m = d.mapNext()) {
+ ensureCollectionAvailable(d, m, minBytes);
r.resize(r.size() + m);
Review Comment:
r.resize(r.size() + m) can overflow size_t before the resize call when m is
very large, potentially wrapping to a small size and causing incorrect decoding
behavior. Add a max_size()-based guard before resizing the map container.
--
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]
