fix ssl MSG_PEEK by youknowone · Pull Request #6892 · RustPython/RustPython

youknowone

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds socket-peek-based TLS shutdown/read logic to the SSL module to distinguish TLS records from post-TLS cleartext during close-notify handling, plus a private helper to consume exactly one TLS record when appropriate. Also adds tests tracking comparison calls in sorting.

Changes

Cohort / File(s)	Summary
TLS socket & TLS shutdown logic `crates/stdlib/src/ssl.rs`	Added `sock_peek` (pub(crate)) and `try_read_close_notify_socket` (private). Updated non-BIO shutdown/read paths to use MSG_PEEK to detect TLS records vs post-TLS cleartext, consume exactly one TLS record when needed, and invoke `process_new_packets` after reading TLS data. Adjusted socket recv/send usage in shutdown flows.
Sorting comparison tests `extra_tests/snippets/builtin_list.py`	Added `TrackComparison` helper and tests asserting `sorted()`/`list.sort()` use `__lt__` (not `__gt__`), including `reverse=True` cases; updated assertion formatting.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant PySSLSocket
    participant Socket
    participant TlsConnection
    participant VM

    Client->>Socket: send data (may be TLS record or cleartext)
    PySSLSocket->>Socket: sock_peek(size)  -- MSG_PEEK
    Socket-->>PySSLSocket: peeked bytes
    PySSLSocket->>TlsConnection: analyze peeked bytes (record header)
    alt TLS record present
        PySSLSocket->>Socket: sock_recv(exact_record_len)
        Socket-->>PySSLSocket: TLS record bytes
        PySSLSocket->>TlsConnection: feed bytes, process_new_packets()
        TlsConnection-->>PySSLSocket: decrypted/plain data
        PySSLSocket->>VM: return decrypted data / handle close-notify
    else Post-TLS cleartext
        PySSLSocket-->>VM: treat peeked bytes as cleartext (do not consume TLS)
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

fix ssl MSG_PEEK #6892: Modifies same ssl.rs helpers and introduces peek-based TLS shutdown/read logic.
better ssl write handling #6763: Overlaps on PySSLSocket I/O and shutdown/read handling around close-notify and process_new_packets.
Share more ssl consts and fix openssl #6462: Related changes to SSL read/shutdown behavior and BIO vs socket handling.

Suggested reviewers

ShaharNaveh

Poem

"I'm a rabbit with a curious peek,
I nibble bytes but never take a sweep—
Close-notify whispers, I listen right,
I pass along records, keep the rest light.
Hooray for sorts that choose lt to leap!"

🚥 Pre-merge checks | ✅ 3

✅ 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 'fix ssl MSG_PEEK' directly corresponds to the primary change: implementing MSG_PEEK-based socket peeking for SSL/TLS shutdown handling.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

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