Skip to content

Fix binascii.b2a_qp CRLF boundary check - #8358

Merged
youknowone merged 1 commit into
RustPython:mainfrom
YangSiJun528:fix-b2a-qp-crlf-check
Jul 25, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
YangSiJun528:fix-b2a-qp-crlf-check

Conversation

@YangSiJun528

@YangSiJun528 YangSiJun528 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Prevent binascii.b2a_qp() from panicking when the input starts with \n.
  • Match CPython's boundary check when detecting CRLF line endings.

When the first byte is \n, in_idx remains zero and accessing buf[in_idx - 1] causes an integer underflow.
This change checks in_idx > 0 before accessing the preceding byte.

Testing

  • cargo run -- -c 'import binascii; assert binascii.b2a_qp(b"\n") == b"\n"'
  • cargo run --release -- -m test test_binascii

Before this change, the RustPython command panicked with:

attempt to subtract with overflow

AI assistance

Codex (GPT‑5.6 Sol) was used to investigate and implement this change.
I reviewed and validated all AI-assisted work.

Summary by CodeRabbit

  • Bug Fixes
    • Improved quoted-printable encoding’s carriage-return detection to avoid incorrect line-ending handling near buffer boundaries.
    • Added tighter boundary checks so CR detection only occurs when the previous byte is safely accessible.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 01a403e0-44b8-40bd-96ee-7e4bb61f4faf

📥 Commits

Reviewing files that changed from the base of the PR and between 5c7950f and d73df27.

📒 Files selected for processing (1)
  • crates/stdlib/src/binascii.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/stdlib/src/binascii.rs

📝 Walkthrough

Walkthrough

b2a_qp now checks that the input index is greater than zero before inspecting the preceding byte when detecting CRLF line endings.

Changes

Quoted-printable encoding

Layer / File(s) Summary
Guard CRLF detection
crates/stdlib/src/binascii.rs
Updates the CRLF detection condition to use in_idx > 0 && in_idx < buflen before accessing buf[in_idx - 1].

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the main fix to the CRLF boundary check in binascii.b2a_qp.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Assisted-by: Codex:gpt-5.6-sol
@YangSiJun528
YangSiJun528 force-pushed the fix-b2a-qp-crlf-check branch from 5c7950f to d73df27 Compare July 24, 2026 11:50

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@crates/stdlib/src/binascii.rs`:
- Line 507: Preserve the in_idx > 0 guard in the CRLF handling condition to
prevent out-of-bounds indexing, then run cargo fmt --all -- --check with
cargo-fmt available and run Clippy, resolving any warnings before merge.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0470bf12-b8f8-45fd-a845-740ac811da17

📥 Commits

Reviewing files that changed from the base of the PR and between 28454cc and 5c7950f.

📒 Files selected for processing (1)
  • crates/stdlib/src/binascii.rs

Comment thread crates/stdlib/src/binascii.rs
@moreal moreal added the z-ca-2026 Tag to track Contribution Academy 2026 label Jul 24, 2026
@youknowone
youknowone merged commit d155c93 into RustPython:main Jul 25, 2026
26 checks passed
youknowone pushed a commit that referenced this pull request Sep 16, 2026
Assisted-by: Codex:gpt-5.6-sol
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants