You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "friendlymatthew (via GitHub)" <gi...@apache.org> on 2025/06/25 22:18:23 UTC
[I] [Variant] Field lookup with out of bounds index causes unwanted behavior [arrow-rs]
friendlymatthew opened a new issue, #7784:
URL: https://github.com/apache/arrow-rs/issues/7784
- Part of https://github.com/apache/arrow-rs/issues/6736
- From https://github.com/apache/arrow-rs/pull/7757#discussion_r2162822572
**Describe the bug**
`VariantObject::field_name(i:usize)` returns the field name corresponding to its index. If index is out of bounds, the method should return `None`, but when the index is the length of the fields, it will return the first element. When the index is larger than the length of the fields, it starts to panic.
**Steps to reproduce**
Please use the following test:
https://github.com/apache/arrow-rs/blob/7d3a25ad7f57854c4ea14aa1f9c63b5717ed8ac9/parquet-variant/src/variant/object.rs#L205-L256
This test builds up a `VariantObject` with three fields: `{"active": true, "age": 42, "name": "hello"}`
```rs
let res = variant_object.field_name(3); // 3 is out of bounds!
// the assertion below _will_ fail. We expect `res` to be `None`, but it returns the first field ("active").
assert_eq!(res.is_none());
// this call below will panic! This is because `field_name` internally calls
// `try_field_name`, which will err with the following:
// validation error after construction: InvalidArgumentError("Tried to extract byte(s) 302..303 from 18-byte buffer")
let res = variant_object.field_name(300); // 300 is definitely out of bounds
```
Note, using the `VariantBuilder` to build the same object and then trying to access fields with an out of bounds index will err correctly:
<details>
<summary>Reproducible test</summary>
```rust
#[test]
fn test_foo() {
let mut variant_builder = VariantBuilder::new();
let mut obj_builder = variant_builder.new_object();
obj_builder.append_value("active", true);
obj_builder.append_value("age", 42);
obj_builder.append_value("name", "hello");
obj_builder.finish();
let (m, v) = variant_builder.finish();
let variant = Variant::try_new(&m, &v).unwrap();
let variant_obj = variant.as_object().unwrap();
let res = variant_obj.field(3);
assert!(res.is_none());
let res = variant_obj.field(300);
assert!(res.is_none());
}
```
</details
But since the failing test should contain valid metadata and value buffers, it is important to investigate why this is occurring here.
**Things we've considered**
You can obviously add a oob check since we know the number of elements in a `VariantObject`, but it seems like there is a bug in the underlying `try_field_name` logic.
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [Variant] Field lookup with out of bounds index causes unwanted behavior [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #7784: [Variant] Field lookup with out of bounds index causes unwanted behavior
URL: https://github.com/apache/arrow-rs/issues/7784
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [Variant] Field lookup with out of bounds index causes unwanted behavior [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on issue #7784:
URL: https://github.com/apache/arrow-rs/issues/7784#issuecomment-3006883451
> ```rust
> // this call below will panic! This is because `field_name` internally calls
> // `try_field_name`, which will err with the following:
> // validation error after construction: InvalidArgumentError("Tried to extract byte(s) 302..303 from 18-byte buffer")
> let res = variant_object.field_name(300); // 300 is definitely out of bounds
> ```
And herein lies the danger of making those methods "infallible" -- if they ever use any code the iterator pass in the constructor did not exercise, they risk a panic. Here I would assume we need a bounds check in the `field_name` method, to ensure the `try_field_name` call only receives arguments the iterator validated.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [Variant] Field lookup with out of bounds index causes unwanted behavior [arrow-rs]
Posted by "friendlymatthew (via GitHub)" <gi...@apache.org>.
friendlymatthew commented on issue #7784:
URL: https://github.com/apache/arrow-rs/issues/7784#issuecomment-3006405724
I'll take a look at this either this weekend or next week. But anyone should feel free to take this issue
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [Variant] Field lookup with out of bounds index causes unwanted behavior [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7784:
URL: https://github.com/apache/arrow-rs/issues/7784#issuecomment-3130010122
`label_issue.py` automatically added labels {'parquet'} from #7807
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [Variant] Field lookup with out of bounds index causes unwanted behavior [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7784:
URL: https://github.com/apache/arrow-rs/issues/7784#issuecomment-3025027432
THis might be fixed after
- https://github.com/apache/arrow-rs/pull/7807
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org