sqlite3: fix Blob.__setitem__ value range validation#7981
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR tightens BLOB setitem: single-index assignments now require a PyInt converted to i64 and must be 0–255; non-unit-step extended-slice assignments preserve unaffected bytes by reading the destination span first. A RustPython-only regression test verifies negative-step slice behavior. ChangesBLOB item assignment improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] lib: cpython/Lib/sqlite3 dependencies:
dependent tests: (2 tests)
Legend:
|
Previously, assigning an out-of-range integer (negative or > 255) or an
integer too large for i64 (e.g. 2**65) to a Blob index raised OverflowError
instead of ValueError.
Mirror CPython's ass_subscript_index logic:
- Convert the value via to_i64(), treating any overflow as -1
- Validate the result is in [0, 255], raising ValueError("byte must be in range(0, 256)") otherwise
- Separate deletion error messages: "item deletion" for index, "slice deletion" for slice
In the step != 1 branch of Blob.ass_subscript, the loop used i_in_temp += step as usize where step is isize. For negative steps (e.g. step = -2), (-2isize) as usize = 18446744073709551614 causing an out-of-bounds panic whenever slice_len >= 2. Fix: use SaturatedSliceIter (already used by the read path) to iterate over the correct absolute blob indices, then map each index back to a temp buffer offset via abs_idx - range_start. Also fix a Clippy lint: replace val < 0 || val > 255 with the idiomatic !(0..=255).contains(&val) Add a regression test in extra_tests/snippets/stdlib_sqlite.py that exercises blob[9:0:-2] (negative step, slice_len=5).
ac57ee6 to
6f3eae8
Compare
Why the snippet test is guarded by
|
Previously, assigning an out-of-range integer (negative or > 255) or an integer too large for i64 (e.g. 2**65) to a Blob index raised OverflowError instead of ValueError.
Mirror CPython's ass_subscript_index logic:
Ref: https://github.com/python/cpython/blob/main/Modules/_sqlite/blob.c
Problem
When assigning an out-of-range integer to a
Blobindex, RustPython raisedOverflowErrorinstead ofValueError:CPython raises
ValueError: byte must be in range(0, 256)for all three cases.Root Cause
The previous implementation used
try_to_primitive::<u8>(vm)?to convert the assigned value, which calls Rust's type conversion and raises a genericOverflowErrorfor any value that doesn't fit inu8— including negative numbers.Fix
Mirrors CPython's
ass_subscript_indexlogic:In Rust:
try_to_primitive::<u8>()withas_bigint().to_i64().unwrap_or(-1)i64(e.g.2**65) are treated as-1, which then fails the range checkValueError("byte must be in range(0, 256)")for any value outside[0, 255]Additionally, aligned the deletion error messages with CPython:
del blob[0]→"Blob doesn't support item deletion"del blob[5:10]→"Blob doesn't support slice deletion"Changes
Blob::ass_subscriptindex branch@unittest.expectedFailurefromtest_blob_set_item_errorSummary by CodeRabbit
Bug Fixes
Tests