c-grellois opened a new issue, #11623:
URL: https://github.com/apache/arrow-rs/issues/11623
## Describe the bug
The `arrow-csv` `Reader`/`Decoder` implements `Iterator<Item =
Result<RecordBatch, ArrowError>>` and is expected to surface parse problems as
`Err(ArrowError)`, not to panic. For a column typed `Dictionary(<small int
key>, Utf8)`, when the number of distinct non-null string values in a single
`batch_size` window exceeds what the key type can address, the reader
**panics** (`dictionary key overflow: DictionaryKeyOverflowError`) instead of
returning `Some(Err(ArrowError::DictionaryKeyOverflowError))`.
This is the CSV-reader sibling of the already-recognised-and-fixed
interleave/concat panic of the same class (#7466, #10674, fixed by PR #10675
"don't panic on dictionary key overflow in interleave/concat") — a path PR
#10675 did not touch. (It is distinct from the `MapBuilder` offset-overflow
issue; this one is in the CSV reader's dictionary-collect path.)
## Reproduction
```rust
use std::sync::Arc;
use arrow_schema::{DataType, Field, Schema};
use arrow_csv::ReaderBuilder;
let schema = Arc::new(Schema::new(vec![Field::new(
"col",
DataType::Dictionary(Box::new(DataType::Int8), Box::new(DataType::Utf8)),
false,
)]));
// 129 distinct non-null values in one batch (Int8 addresses only 128: keys
0..=127)
let mut csv = String::from("col\n");
for i in 0..129 { csv.push_str(&format!("v{i}\n")); }
let mut rdr = ReaderBuilder::new(schema)
.with_header(true)
.with_batch_size(1024)
.build(std::io::Cursor::new(csv.into_bytes()))
.unwrap();
let _ = rdr.next(); // expected: Some(Err(DictionaryKeyOverflowError));
actual: panics
```
`Cargo.toml`: `arrow-csv = "=60.0.0"`, `arrow-schema = "=60.0.0"`. Observed
on **60.0.0**:
```
thread 'main' panicked at
arrow-array-60.0.0/src/builder/generic_bytes_dictionary_builder.rs:332:
dictionary key overflow: DictionaryKeyOverflowError
```
Controls (confirming the boundary, not an off-by-one): `Int8` + **128**
distinct → `Ok`; `Int8` + **129** → **panic**; `Int32` + 129 → `Ok` (same
input, wider key).
## Expected vs actual
- **Expected:** `Reader::next()` yields
`Some(Err(ArrowError::DictionaryKeyOverflowError))` — the surrounding CSV code
already returns `Err(ParseError(...))` for other bad input, including an
unsupported dictionary key type.
- **Actual:** the parse thread panics and unwinds.
## Root cause
`arrow-csv/src/reader/mod.rs` (`parse`, the `DataType::Dictionary(<int key>,
Utf8)` arms) builds the column with `collect::<DictionaryArray<K>>()` wrapped
in `Ok(Arc::new(...))` — no `?`, no `map_err`. That goes through `FromIterator
→ GenericByteDictionaryBuilder::extend → append_option → append_value`, whose
body is `self.append(value).expect("dictionary key overflow")`
(`generic_bytes_dictionary_builder.rs:332`, 60.0.0). The fallible `append`
(which returns `Err(DictionaryKeyOverflowError)`) exists, but the infallible
`append_value` turns the overflow into a panic, and the CSV arm never routes
through the error path. Affects the small-key arms: `Int8`/`UInt8` (past 128 /
256 distinct) and `Int16`/`UInt16` (past 32768 / 65536); `Int32`/`Int64` are
effectively immune.
## Suggested fix
In the CSV reader's dictionary arms, build with the fallible path and
propagate the error — e.g. use `append`/`try_extend` (or `try_collect` into the
builder and `?` / `map_err` the result) so a key overflow becomes
`Err(ArrowError::DictionaryKeyOverflowError)` instead of a panic, mirroring PR
#10675 for interleave/concat.
## Impact and scope
Low / borderline-informational. An integrator who declares a small-key
`Dictionary(Int8/Int16/…, Utf8)` schema (a legitimate low-cardinality
optimisation) and parses attacker- or user-supplied CSV gets a parse-thread
panic instead of the documented `Result::Err` — a
parser-robustness/availability bug, no out-of-bounds read and no
confidentiality/integrity impact. Not reachable under default schema inference
(which never emits dictionary types) or for Int32/Int64 keys. CVSS:3.1
`AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:L` = 3.7 (Low); treat as a robustness issue
rather than a security advisory.
## Classification
- **CWE-248** (Uncaught Exception / panic) — primary; **CWE-755** (Improper
Handling of Exceptional Conditions). (Not CWE-125 — there is no out-of-bounds
read; the builder cleanly detects key-space exhaustion.)
- **Affected:** `arrow`/`arrow-csv`/`arrow-array` through **60.0.0**
(current) and `main`; pattern present across 57.x–60.x. **Patched:** none.
--
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]
