Fix SSL test_preauth_data_to_tls_server by youknowone · Pull Request #6508 · RustPython/RustPython

youknowone

📝 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 docstrings

🧪 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.}