You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "efegokdemir (via GitHub)" <gi...@apache.org> on 2026/09/23 18:03:42 UTC
[PR] GH-1261: Reject out-of-range dictionary indices [arrow-java]
efegokdemir opened a new pull request, #1309:
URL: https://github.com/apache/arrow-java/pull/1309
## What's Changed
Reject negative and dictionaryCount indices before copying dictionary values.
Add regression coverage for both invalid bounds and allocator cleanup, and keep the existing struct decode regression focused on the later invalid field.
Closes #1261.
AI assistance was used during development; the complete diff and test results were reviewed by the author.
--
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: [PR] GH-1261: Reject out-of-range dictionary indices [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #1309:
URL: https://github.com/apache/arrow-java/pull/1309#discussion_r4173064020
##########
vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryEncoder.java:
##########
@@ -165,7 +165,7 @@ static void retrieveIndexVector(
for (int i = start; i < end; i++) {
if (!indices.isNull(i)) {
int indexAsInt = (int) indices.getValueAsLong(i);
- if (indexAsInt > dictionaryCount) {
+ if (indexAsInt < 0 || indexAsInt >= dictionaryCount) {
Review Comment:
This bound is correct, but one of the three callers doesn't pass the dictionary size. `StructSubfieldEncore.decode` does:
```java
DictionaryEncoder.retrieveIndexVector(indices, transfer, valueCount, 0, valueCount);
```
where `valueCount` is the struct row count. So for the struct path:
- a 5-row strict with a 2-entry dictionary and an index of 4 still passes the check and reads past the dictionary, which is the case #1261 describes
- a 1-row struct with index 1 into a 2-entry dictionary used to decode (`1 > 1` is false) and now throws (`1 >= 1`), although the index is valid
Could you pass the dictionary count there?
```java
DictionaryEncoder.retrieveIndexVector(
indices, transfer, dictionary.getVector().getValueCount(), 0, valueCount);
```
Alternatively, `retrieveIndexVector` could take the dictionary vector and call `getValueCount()` itself, so callers can't pass the wrong number.
@lidavidm thoughts?
--
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: [PR] GH-1261: Reject out-of-range dictionary indices [arrow-java]
Posted by "efegokdemir (via GitHub)" <gi...@apache.org>.
efegokdemir commented on PR #1309:
URL: https://github.com/apache/arrow-java/pull/1309#issuecomment-5981425221
Fixed in `167a97ea0cd272136808a00d907da411d233b364`. `retrieveIndexVector` now checks the original `long` before narrowing, and the BigIntVector regression covers both a valid index and `1L << 32`, asserting the full out-of-range value in the exception. `git diff --check` passed. The focused Maven test and Spotless check are blocked locally because the Develocity Maven extension fails during initialization (`NullInjectedIntoNonNullable`), before goals execute. GitHub Ensure PR format passed; test/RC checks are running.
--
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: [PR] GH-1261: Reject out-of-range dictionary indices [arrow-java]
Posted by "efegokdemir (via GitHub)" <gi...@apache.org>.
efegokdemir commented on code in PR #1309:
URL: https://github.com/apache/arrow-java/pull/1309#discussion_r4173120822
##########
vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryEncoder.java:
##########
@@ -165,7 +165,7 @@ static void retrieveIndexVector(
for (int i = start; i < end; i++) {
if (!indices.isNull(i)) {
int indexAsInt = (int) indices.getValueAsLong(i);
- if (indexAsInt > dictionaryCount) {
+ if (indexAsInt < 0 || indexAsInt >= dictionaryCount) {
Review Comment:
Fixed in `4901f9855c370818a41ae1ce42ad3a077d18c364`: struct decoding now passes `dictionary.getVector().getValueCount()` as the dictionary bound while retaining the struct row count as the decode count. Added regression coverage for a valid one-row/index-1 decode against a two-entry dictionary and an out-of-range index in a five-row struct. Local validation: `mvn -pl vector -am -Denforcer.skip=true -DskipITs -Dtest=TestDictionaryVector -Dsurefire.failIfNoSpecifiedTests=false test` (28/28 passed in both Netty and Unsafe allocator runs), `mvn -pl vector -Denforcer.skip=true spotless:check` passed, and `git diff --check` passed. GitHub PR HEAD was fetched and verified to contain both changes. CI on the new HEAD has started.
--
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: [PR] GH-1261: Reject out-of-range dictionary indices [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #1309:
URL: https://github.com/apache/arrow-java/pull/1309
--
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: [PR] GH-1261: Reject out-of-range dictionary indices [arrow-java]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1309:
URL: https://github.com/apache/arrow-java/pull/1309#issuecomment-5800553705
<!-- label_helper_comment -->
Thank you for opening a pull request!
Please label the PR with one or more of:
- bug-fix
- chore
- dependencies
- documentation
- enhancement
Also, add the 'breaking-change' label if appropriate.
See [CONTRIBUTING.md](https://github.com/apache/arrow-java/blob/main/CONTRIBUTING.md) for details.
--
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