Fix str.expandtabs aborting on a tab size of zero - #8562
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesExpandtabs zero-tab-size handling
Empty-pattern replacement coverage
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change prevents process termination for zero or negative tab sizes and adds focused coverage; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
`"a\tb".expandtabs(0)` panicked with a capacity overflow. CPython returns `'ab'`: with no width to advance to, the tabs come out and nothing else moves. `expandtabs(-1)` is the same call, since `ExpandTabsArgs::tabsize` sends every negative value to 0. `expandtabs` keeps the tab stop in `tab_size` and the current column in `col_count`, and on a tab it does `tab_size - col_count`. With a tab size of zero both start at 0, the first character makes `col_count` 1 while `tab_size` stays 0, and the subtraction underflows. The run of spaces asked for next is `usize::MAX`, and the allocation aborts the process. A tab has to follow something on the line to reach it: `"\ta".expandtabs(0)` subtracts 0 from 0 and comes out right by accident. `BytesInner::expandtabs` already returns early for this and filters the tabs out. The string version now does the same. Assisted-by: Claude Code:claude-opus-5
a707729 to
410844f
Compare
"a\tb".expandtabs(0)takes the interpreter down:CPython returns
'ab': with no width to advance to, the tabs come out and nothing else moves.expandtabs(-1)is the same, becauseExpandTabsArgs::tabsizesends every negative value to 0.rustpython_common::str::expandtabswalks the string with the tab stop intab_sizeand the current column incol_count, and on a tab it doestab_size - col_count. Both start at zero, so the first character makescol_count1 whiletab_sizestays 0, and the subtraction underflows. The run of spaces asked for next isusize::MAX, and the allocation aborts the process. That is why a tab has to follow something:"\ta".expandtabs(0)starts the tab at column 0, subtracts 0 from 0, and comes out right by accident.BytesInner::expandtabsalready returns early for this, filtering the tabs out. The string version now does the same thing.Reachable from any Python code, no C API and no unusual build needed:
Checked against CPython 3.14.7 over 23 subjects by 10 tab sizes by
str,bytesandbytearray, 460 cases. All 460 agree now. Without the change 42 of them abort the process, and thebyteshalf is not among them, which is where the shape of the fix came from.Tests are in
crates/common/src/str.rsnext to the function and inextra_tests/snippets/builtin_str.pyalongside theexpandtabsoverflow cases that landed in #8524. Both cover the zero and negative sizes, ASCII and non-ASCII, tabs after a newline and after a carriage return, and a set of ordinary tab sizes so a fix that reached too far would show up.Lib/test/string_tests.pyhas no case for a zero tab size, sotest_strandtest_bytespass either way. They still pass here, along withtest_textwrap, 523 tests.Summary by CodeRabbit
Bug Fixes
expandtabshandling for zero tab sizes, preventing excessive memory allocation and process termination.Tests