csv: reject reentrant reader advancement - #8324

crates/stdlib/src/csv.rs (1)

1062-1093: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Increment generation unconditionally immediately after the reentrancy check.

Delaying the generation increment to successful read paths allows an inner reentrant call that mutates the parser state but subsequently errors out (e.g. due to an invalid newline) to bypass the generation update. The outer call will then fail to detect the reentrancy and proceed with corrupted parser state. Furthermore, performing the reentrancy check after downcasting to PyStr can incorrectly shadow the reentrancy error with a type error.

Consolidate the generation updates to execute exactly once right after acquiring the state lock, mirroring CPython's exact sequence of operations.

• crates/stdlib/src/csv.rs#L1062-L1093: Move the state.generation += 1 to immediately follow the reentrancy check, before downcasting. Update the destructuring to safely ignore the now-handled generation field.

🐛 Proposed fix for the early increment

-            let generation = zelf.state.lock().generation;
-            let string = raise_if_stop!(zelf.iter.next(vm)?);
-            let string = string.downcast::<PyStr>().map_err(|obj| {
-                new_csv_error(
-                    vm,
-                    format!(
-                "iterator should return strings, not {} (the file should be opened in text mode)",
-                obj.class().name()
-            ),
-                )
-            })?;
-            let input = string.as_bytes();
+            let generation = zelf.state.lock().generation;
+            let string_obj = raise_if_stop!(zelf.iter.next(vm)?);
+            
             let mut state = zelf.state.lock();
             if state.generation != generation {
                 return Err(new_csv_error(
                     vm,
                     "iterator has already advanced the reader",
                 ));
             }
+            state.generation += 1;
+
+            let string = string_obj.downcast::<PyStr>().map_err(|obj| {
+                new_csv_error(
+                    vm,
+                    format!(
+                        "iterator should return strings, not {} (the file should be opened in text mode)",
+                        obj.class().name()
+                    ),
+                )
+            })?;
+            let input = string.as_bytes();
+
             if input.is_empty() || input.starts_with(b"\n") {
-                state.generation += 1;
                 return Ok(PyIterReturn::Return(vm.ctx.new_list(vec![]).into()));
             }
             let ReadState {
                 buffer,
                 output_ends,
                 reader,
                 skipinitialspace,
                 delimiter,
                 line_num,
-                generation,
+                generation: _,
             } = &mut *state;

• crates/stdlib/src/csv.rs#L1100-L1105: Remove the now-redundant *generation += 1;.

🐛 Proposed fix for the `QUOTE_NONE` branch

             if zelf.dialect.quoting == QuoteStyle::None && zelf.dialect.escapechar.is_some() {
                 let out = read_quote_none_record(input, zelf.dialect, field_limit, vm)?;
                 *line_num += 1;
-                *generation += 1;
                 return Ok(PyIterReturn::Return(vm.ctx.new_list(out).into()));
             }

• crates/stdlib/src/csv.rs#L1188-L1192: Remove the now-redundant *generation += 1;.

🐛 Proposed fix for the standard branch

             // if out.last().unwrap().length(vm).unwrap() == 0 {
             //     out.pop();
             // }
             *line_num += 1;
-            *generation += 1;
             Ok(PyIterReturn::Return(vm.ctx.new_list(out).into()))

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/stdlib/src/csv.rs` around lines 1062 - 1093, Update
crates/stdlib/src/csv.rs:1062-1093 in the iterator read flow to increment
state.generation exactly once immediately after the reentrancy check and before
PyStr downcasting; destructure ReadState while safely ignoring the
already-handled generation field. Remove the redundant generation increments in
the QUOTE_NONE branch at crates/stdlib/src/csv.rs:1100-1105 and the standard
branch at crates/stdlib/src/csv.rs:1188-1192, preserving the existing reentrancy
validation and parsing behavior.

crates/stdlib/src/csv.rs (1)

965-965: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Use u64 to prevent potential integer overflow on 32-bit platforms.

On 32-bit targets (like WebAssembly, which RustPython supports), usize is 32-bit. Iterating through a CSV with more than 4.2 billion rows could cause generation += 1 to panic (in debug builds or release builds with overflow checks enabled). Using u64, similar to line_num, completely mitigates this risk.

🐛 Proposed fix

-        generation: usize,
+        generation: u64,

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/stdlib/src/csv.rs` at line 965, Change the CSV iteration state field
generation from usize to u64, and update its initialization or increment sites
as needed so generation += 1 remains type-correct. Keep the existing
line_num-style counter behavior unchanged.