StatelessIncrementalEncoder, sys attributes#7195
Merged
youknowone merged 3 commits intoRustPython:mainfrom Feb 21, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request introduces stateless codec encoder/decoder wrapper classes as fallback mechanisms in the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_compile.py (TODO: 24) dependencies: dependent tests: (no tests depend on compile) [x] test: cpython/Lib/test/test_sys.py (TODO: 8) dependencies: dependent tests: (213 tests)
Legend:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/vm/src/stdlib/io.rs`:
- Around line 2610-2653: The encode wrapper in StatelessIncrementalEncoder
currently ignores the second tuple item from the codec result; update the encode
method to validate that res.try_into_value(vm)? yields a 2-tuple where the
second element is an integer (non-negative if appropriate to your codec
contract) and raise a TypeError via vm.new_type_error when it is missing or not
an int (reusing the existing error message "encoder must return a tuple (object,
integer)"). Locate the encode method and the tuple variable, convert/validate
tuple[1] as an integer (e.g., via try_into_value or checking .isinstance), and
only then return tuple[0].clone(), ensuring malformed codec outputs surface as
errors.
- Around line 2656-2694: In StatelessIncrementalDecoder::decode, validate that
the second tuple element is an integer and raise a TypeError if not; after
converting res to PyTupleRef and checking tuple.len(), try to convert tuple[1]
to an integer (using the VM's integer/index conversion helpers) and return
Err(vm.new_type_error("decoder must return a tuple (object, integer)")) when
conversion fails, otherwise proceed to return tuple[0].clone() as before. Ensure
this check is applied in the decode method to mirror the encoder wrapper's
guard.
Comment on lines
+2610
to
+2653
| #[pyclass(module = "_io", name, no_attr)] | ||
| #[derive(Debug, PyPayload)] | ||
| struct StatelessIncrementalEncoder { | ||
| encode: PyObjectRef, | ||
| errors: Option<PyStrRef>, | ||
| name: Option<PyStrRef>, | ||
| } | ||
|
|
||
| #[pyclass] | ||
| impl StatelessIncrementalEncoder { | ||
| #[pymethod] | ||
| fn encode( | ||
| &self, | ||
| input: PyObjectRef, | ||
| _final: OptionalArg<bool>, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let mut args: Vec<PyObjectRef> = vec![input]; | ||
| if let Some(errors) = &self.errors { | ||
| args.push(errors.to_owned().into()); | ||
| } | ||
| let res = self.encode.call(args, vm)?; | ||
| let tuple: PyTupleRef = res.try_into_value(vm)?; | ||
| if tuple.len() != 2 { | ||
| return Err(vm.new_type_error("encoder must return a tuple (object, integer)")); | ||
| } | ||
| Ok(tuple[0].clone()) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn reset(&self) {} | ||
|
|
||
| #[pymethod] | ||
| fn setstate(&self, _state: PyObjectRef) {} | ||
|
|
||
| #[pymethod] | ||
| fn getstate(&self, vm: &VirtualMachine) -> PyObjectRef { | ||
| vm.ctx.new_int(0).into() | ||
| } | ||
|
|
||
| #[pygetset] | ||
| fn name(&self) -> Option<PyStrRef> { | ||
| self.name.clone() | ||
| } |
Contributor
There was a problem hiding this comment.
Validate the consumed-length element from codec output.
The wrapper ignores the second tuple item entirely; if it’s not an integer, malformed codecs won’t surface errors. Consider validating it to match the codec contract.
🛠️ Suggested guard for the consumed-length slot
let tuple: PyTupleRef = res.try_into_value(vm)?;
if tuple.len() != 2 {
return Err(vm.new_type_error("encoder must return a tuple (object, integer)"));
}
+ let _consumed: isize = isize::try_from_object(vm, tuple[1].clone()).map_err(|_| {
+ vm.new_type_error("encoder must return a tuple (object, integer)")
+ })?;
Ok(tuple[0].clone())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[pyclass(module = "_io", name, no_attr)] | |
| #[derive(Debug, PyPayload)] | |
| struct StatelessIncrementalEncoder { | |
| encode: PyObjectRef, | |
| errors: Option<PyStrRef>, | |
| name: Option<PyStrRef>, | |
| } | |
| #[pyclass] | |
| impl StatelessIncrementalEncoder { | |
| #[pymethod] | |
| fn encode( | |
| &self, | |
| input: PyObjectRef, | |
| _final: OptionalArg<bool>, | |
| vm: &VirtualMachine, | |
| ) -> PyResult { | |
| let mut args: Vec<PyObjectRef> = vec![input]; | |
| if let Some(errors) = &self.errors { | |
| args.push(errors.to_owned().into()); | |
| } | |
| let res = self.encode.call(args, vm)?; | |
| let tuple: PyTupleRef = res.try_into_value(vm)?; | |
| if tuple.len() != 2 { | |
| return Err(vm.new_type_error("encoder must return a tuple (object, integer)")); | |
| } | |
| Ok(tuple[0].clone()) | |
| } | |
| #[pymethod] | |
| fn reset(&self) {} | |
| #[pymethod] | |
| fn setstate(&self, _state: PyObjectRef) {} | |
| #[pymethod] | |
| fn getstate(&self, vm: &VirtualMachine) -> PyObjectRef { | |
| vm.ctx.new_int(0).into() | |
| } | |
| #[pygetset] | |
| fn name(&self) -> Option<PyStrRef> { | |
| self.name.clone() | |
| } | |
| #[pyclass(module = "_io", name, no_attr)] | |
| #[derive(Debug, PyPayload)] | |
| struct StatelessIncrementalEncoder { | |
| encode: PyObjectRef, | |
| errors: Option<PyStrRef>, | |
| name: Option<PyStrRef>, | |
| } | |
| #[pyclass] | |
| impl StatelessIncrementalEncoder { | |
| #[pymethod] | |
| fn encode( | |
| &self, | |
| input: PyObjectRef, | |
| _final: OptionalArg<bool>, | |
| vm: &VirtualMachine, | |
| ) -> PyResult { | |
| let mut args: Vec<PyObjectRef> = vec![input]; | |
| if let Some(errors) = &self.errors { | |
| args.push(errors.to_owned().into()); | |
| } | |
| let res = self.encode.call(args, vm)?; | |
| let tuple: PyTupleRef = res.try_into_value(vm)?; | |
| if tuple.len() != 2 { | |
| return Err(vm.new_type_error("encoder must return a tuple (object, integer)")); | |
| } | |
| let _consumed: isize = isize::try_from_object(vm, tuple[1].clone()).map_err(|_| { | |
| vm.new_type_error("encoder must return a tuple (object, integer)") | |
| })?; | |
| Ok(tuple[0].clone()) | |
| } | |
| #[pymethod] | |
| fn reset(&self) {} | |
| #[pymethod] | |
| fn setstate(&self, _state: PyObjectRef) {} | |
| #[pymethod] | |
| fn getstate(&self, vm: &VirtualMachine) -> PyObjectRef { | |
| vm.ctx.new_int(0).into() | |
| } | |
| #[pygetset] | |
| fn name(&self) -> Option<PyStrRef> { | |
| self.name.clone() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/stdlib/io.rs` around lines 2610 - 2653, The encode wrapper in
StatelessIncrementalEncoder currently ignores the second tuple item from the
codec result; update the encode method to validate that res.try_into_value(vm)?
yields a 2-tuple where the second element is an integer (non-negative if
appropriate to your codec contract) and raise a TypeError via vm.new_type_error
when it is missing or not an int (reusing the existing error message "encoder
must return a tuple (object, integer)"). Locate the encode method and the tuple
variable, convert/validate tuple[1] as an integer (e.g., via try_into_value or
checking .isinstance), and only then return tuple[0].clone(), ensuring malformed
codec outputs surface as errors.
Comment on lines
+2656
to
+2694
| #[pyclass(module = "_io", name, no_attr)] | ||
| #[derive(Debug, PyPayload)] | ||
| struct StatelessIncrementalDecoder { | ||
| decode: PyObjectRef, | ||
| errors: Option<PyStrRef>, | ||
| } | ||
|
|
||
| #[pyclass] | ||
| impl StatelessIncrementalDecoder { | ||
| #[pymethod] | ||
| fn decode( | ||
| &self, | ||
| input: PyObjectRef, | ||
| _final: OptionalArg<bool>, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let mut args: Vec<PyObjectRef> = vec![input]; | ||
| if let Some(errors) = &self.errors { | ||
| args.push(errors.to_owned().into()); | ||
| } | ||
| let res = self.decode.call(args, vm)?; | ||
| let tuple: PyTupleRef = res.try_into_value(vm)?; | ||
| if tuple.len() != 2 { | ||
| return Err(vm.new_type_error("decoder must return a tuple (object, integer)")); | ||
| } | ||
| Ok(tuple[0].clone()) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn getstate(&self, vm: &VirtualMachine) -> (PyBytesRef, u64) { | ||
| (vm.ctx.empty_bytes.to_owned(), 0) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn setstate(&self, _state: PyTupleRef, _vm: &VirtualMachine) {} | ||
|
|
||
| #[pymethod] | ||
| fn reset(&self) {} | ||
| } |
Contributor
There was a problem hiding this comment.
Validate the consumed-length element from codec output.
Same concern as the encoder wrapper: the second tuple element should be an integer; otherwise invalid codec implementations pass silently.
🛠️ Suggested guard for the consumed-length slot
let tuple: PyTupleRef = res.try_into_value(vm)?;
if tuple.len() != 2 {
return Err(vm.new_type_error("decoder must return a tuple (object, integer)"));
}
+ let _consumed: isize = isize::try_from_object(vm, tuple[1].clone()).map_err(|_| {
+ vm.new_type_error("decoder must return a tuple (object, integer)")
+ })?;
Ok(tuple[0].clone())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/stdlib/io.rs` around lines 2656 - 2694, In
StatelessIncrementalDecoder::decode, validate that the second tuple element is
an integer and raise a TypeError if not; after converting res to PyTupleRef and
checking tuple.len(), try to convert tuple[1] to an integer (using the VM's
integer/index conversion helpers) and return Err(vm.new_type_error("decoder must
return a tuple (object, integer)")) when conversion fails, otherwise proceed to
return tuple[0].clone() as before. Ensure this check is applied in the decode
method to mirror the encoder wrapper's guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features