GeezerVison is showing github.com — Fix SSL test_preauth_data_to_tls_server by youknowone · Pull Request #6508 · RustPython/RustPython
Conversation
youknowone
commented
Dec 25, 2025
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary by CodeRabbit
- Bug Fixes
- Improved error detection and reporting for non-TLS data received during SSL/TLS handshake initiation, providing clearer error messages when connection issues occur.
✏️ Tip: You can customize this high-level summary in your review settings.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Dec 25, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
📝 Walkthrough
Walkthrough
A new PreauthData error variant was added to the SslError enum to represent non-TLS data received before TLS handshake completion. The conversion logic from rustls errors and to Python exceptions was updated to handle this new variant.
Changes
| Cohort / File(s) |
Summary |
SSL Error Variant Addition
crates/stdlib/src/ssl/compat.rs |
Added PreauthData variant to SslError enum; mapped rustls::Error::InvalidMessage to SslError::PreauthData during error conversion; added PreauthData arm in SslError-to-Python exception conversion to produce SSLError with library=None and reason "before TLS handshake with data" |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
- Fix SSL error handling #6351 — Modifies
create_ssl_error_with_reason to accept optional library parameter, which is used by this PR when converting PreauthData to Python exceptions with library=None
- Fix install ujson #6502 — Adds another
SslError variant (Timeout) in the same file using similar rustls-to-SslError and SslError-to-Python conversion patterns
- Fix openssl build and shared ssl/error.rs #6456 — Modifies SSL error handling and exception conversion paths in the same
ssl/compat.rs code
Poem
🐰 Before the handshake blooms so bright,
Non-TLS whispers in the night,
PreauthData catches them with care,
Mapping rustls errors through the air—
All with library set to nil, fair!
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
| Check name |
Status |
Explanation |
| Description Check |
✅ Passed |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check |
✅ Passed |
The title accurately describes the main change: fixing SSL pre-authentication data handling by adding a new SslError variant and related conversions. |
| Docstring Coverage |
✅ Passed |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✨ Finishing touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
📜 Recent review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between aae6bf5 and f12a547.
📒 Files selected for processing (1)
crates/stdlib/src/ssl/compat.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/stdlib/src/ssl/compat.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run snippets and cpython tests (windows-latest)
🔇 Additional comments (3)
crates/stdlib/src/ssl/compat.rs (3)
394-395: LGTM! Clean error variant addition.
The new PreauthData variant is well-documented and follows Rust conventions. The placement within the enum is logical, grouping it with other protocol-level errors.
567-575: LGTM! Consistent error conversion implementation.
The conversion of PreauthData to a Python exception follows the established pattern in this codebase. Setting library=None is appropriate for this protocol-level error, consistent with the Syscall error handling.
1259-1263: The code correctly maps rustls::Error::InvalidMessage to SslError::PreauthData during handshake. The InvalidMessage error during the TLS handshake phase reliably indicates non-TLS data (such as HTTP requests sent to TLS servers), as valid TLS ClientHello/ServerHello messages would parse successfully. The distinct handling from the general case (which maps InvalidMessage to Eof via from_rustls) is intentional: during handshake, any parsing failure indicates pre-authentication data before TLS began; after handshake, it indicates a protocol violation. The error message "before TLS handshake with data" and the PreauthData enum documentation (line 395: "Non-TLS data received before handshake completed") accurately reflect this distinction.
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 and usage tips.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.