Copilot commented on code in PR #3433:
URL: https://github.com/apache/avro/pull/3433#discussion_r2454848842
##########
lang/c++/impl/FileStream.cc:
##########
@@ -205,8 +205,15 @@ class BufferCopyInInputStream : public SeekableInputStream
{
void seek(int64_t position) final {
// BufferCopyIn::seek is relative to byteCount_, whereas position is
// absolute.
- in_->seek(position - byteCount_ - available_);
- byteCount_ = position;
+ int64_t offset = position - static_cast<int64_t>(byteCount_) -
static_cast<int64_t>(available_);
+ if (offset < 0) {
+ throw Exception("Negative offset in seek");
+ }
+ in_->seek(static_cast<size_t>(offset));
+ if (position < 0) {
+ throw Exception("Negative position not allowed");
+ }
Review Comment:
The negative position check should occur at the beginning of the function,
before any calculations are performed with `position`. Moving this validation
earlier would fail fast and avoid unnecessary computation.
##########
lang/c++/impl/FileStream.cc:
##########
@@ -205,8 +205,15 @@ class BufferCopyInInputStream : public SeekableInputStream
{
void seek(int64_t position) final {
// BufferCopyIn::seek is relative to byteCount_, whereas position is
// absolute.
- in_->seek(position - byteCount_ - available_);
- byteCount_ = position;
+ int64_t offset = position - static_cast<int64_t>(byteCount_) -
static_cast<int64_t>(available_);
+ if (offset < 0) {
+ throw Exception("Negative offset in seek");
+ }
+ in_->seek(static_cast<size_t>(offset));
+ if (position < 0) {
+ throw Exception("Negative position not allowed");
+ }
Review Comment:
The negative offset check occurs after the subtraction, which means if
`position` is less than `byteCount_ + available_`, a negative offset will be
computed and then converted to a very large `size_t` value when cast on line
212. The check should occur before the cast, but the cast happens inside the
function call. Consider restructuring to avoid passing a potentially negative
value through the cast, or ensure the arithmetic cannot produce negative
results through earlier validation.
```suggestion
if (position < 0) {
throw Exception("Negative position not allowed");
}
int64_t offset = position - static_cast<int64_t>(byteCount_) -
static_cast<int64_t>(available_);
if (offset < 0) {
throw Exception("Negative offset in seek");
}
in_->seek(static_cast<size_t>(offset));
```
--
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]
