stdlib/Cargo.toml (1)

43-43: phf addition looks fine; consider gating under ssl feature to avoid extra deps for non‑SSL builds.

Move phf under an optional dependency and include it in the existing ssl feature to cut compile time for builds without SSL.

Example:

-[dependencies]
+[dependencies]
@@
-phf = { version = "0.11", features = ["macros"] }
+phf = { version = "0.11", features = ["macros"], optional = true }
@@
 [features]
 ssl = ["openssl", "openssl-sys", "foreign-types-shared", "openssl-probe"]
+ssl = ["dep:phf", "openssl", "openssl-sys", "foreign-types-shared", "openssl-probe"]

common/src/fileutils.rs (2)

460-471: EINTR retry on Windows is unnecessary.

_wfopen on Windows doesn’t meaningfully return EINTR; looping adds complexity without benefit. Optional cleanup: drop the retry loop in the Windows branch while keeping it on Unix.

---

489-494: Prefer OsStrExt::as_bytes() within Unix-gated code for idiomatic platform-specific handling.

The original MSRV concern is incorrect—as_encoded_bytes() was stabilized in Rust 1.74.0, well before the repository's MSRV of 1.89.0. However, since this code is already guarded with #[cfg(not(windows))], using the Unix-idiomatic OsStrExt::as_bytes() is preferred for direct OS-level byte access within Unix-specific contexts.

    #[cfg(not(windows))]
    {
        // Convert path to CString
+       use std::os::unix::ffi::OsStrExt;
-       let path_cstring = CString::new(path.as_os_str().as_encoded_bytes())
+       let path_cstring = CString::new(path.as_os_str().as_bytes())
            .map_err(|_| std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid path"))?;

stdlib/src/ssl/ssl_data_31.rs (3)

1-6: Add provenance details in header for reproducibility.

Include OpenSSL version/commit and the generator script version to make diffs auditable.

---

69-1763: Large PHF map: consider feature‑gating and/or splitting to reduce compile time for non‑SSL builds.

Keeping data generated and static is fine; gating the module behind ssl keeps builds lean.

---

1766-1769: Deduplicate encode_error_key across versions.

Move this helper to a shared ssl module and re‑export to avoid repetition.

stdlib/src/ssl/ssl_data_111.rs (3)

1-6: Add provenance details in header for reproducibility.

Note OpenSSL 1.1.1 exact tag/commit and generator script version.

---

61-1340: Same suggestion as 3.1: gate and/or split to reduce compile overhead when SSL is disabled.

---

1342-1346: Deduplicate encode_error_key across versions.

stdlib/src/ssl/ssl_data_300.rs (1)

7-14: Add a tiny sanity-test in-tree (optional).

Consider a unit test that spot-checks a few encode_error_key lookups across libs (e.g., SSL 20/134) to catch generator drift.

Also applies to: 66-74

scripts/make_ssl_data_rs.py (2)

71-87: Clarify dedup behavior for lib codes.

Comment says “keep the last one,” but iteration is by sorted(libname), so “alphabetically last name wins,” not “last in source file.” Either adjust the comment or switch to source-order tracking if that was intended.

---

130-137: Fix generator header path (nit).

Header says tools/make_ssl_data_rs.py; file resides under scripts/. Update for accuracy.

stdlib/src/ssl.rs (1)

1408-1417: SNI callback setter: ensure arg cleared when disabling.

When clearing the callback, also clear the tlsext servername arg to avoid dangling context pointers.

Apply this diff:

                 } else {
                     // Clear callback
                     *callback_guard = None;
                     unsafe {
                         sys::SSL_CTX_set_tlsext_servername_callback__fixed_rust(
                             self.ctx().as_ptr(),
                             None,
                         );
+                        sys::SSL_CTX_set_tlsext_servername_arg(self.ctx().as_ptr(), std::ptr::null_mut());
                     }
                 }

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

stdlib/src/ssl.rs (2)

619-619: Critical: Still using hardcoded ex_data index 0 for PySslSocket pointer.

This was flagged in a previous review but remains unaddressed. Using index 0 without allocation risks collisions with other OpenSSL users. While you correctly allocate a dedicated index for SNI callback data at line 540-552, the SSL socket pointer storage at lines 619, 1563, and 1696 still uses hardcoded 0.

Allocate a dedicated ex_data index for storing the PySslSocket pointer, similar to how get_sni_ex_data_index() works:

+    // Get or create an ex_data index for storing PySslSocket*
+    fn get_ssl_socket_ex_data_index() -> libc::c_int {
+        use std::sync::LazyLock;
+        static SSL_SOCKET_EX_DATA_IDX: LazyLock<libc::c_int> = LazyLock::new(|| unsafe {
+            sys::SSL_get_ex_new_index(0, std::ptr::null_mut(), None, None, None)
+        });
+        *SSL_SOCKET_EX_DATA_IDX
+    }

Then replace all three occurrences:

-            let ssl_socket_ptr = sys::SSL_get_ex_data(ssl_ptr, 0); // Index 0 for SSL socket
+            let ssl_socket_ptr = sys::SSL_get_ex_data(ssl_ptr, get_ssl_socket_ex_data_index());

-                    sys::SSL_set_ex_data(ssl_ptr, 0, &*py_ref as *const _ as *mut _);
+                    sys::SSL_set_ex_data(ssl_ptr, get_ssl_socket_ex_data_index(), &*py_ref as *const _ as *mut _);

Also applies to: 1563-1563, 1696-1696

---

1058-1072: cfg gate still incorrect for num_tickets methods.

Despite being marked as "✅ Addressed" in the past review, lines 1061, 1067, 1087, and 1096 still use #[cfg(ossl110)] instead of #[cfg(ossl111)]. The functions SSL_CTX_get_num_tickets and SSL_CTX_set_num_tickets were introduced in OpenSSL 1.1.1, not 1.1.0.

Apply this diff:

-            #[cfg(ossl110)]
+            #[cfg(ossl111)]
             {
                 let ctx = self.ctx();
                 let num = unsafe { sys::SSL_CTX_get_num_tickets(ctx.as_ptr()) };
                 Ok(num)
             }
-            #[cfg(not(ossl110))]
+            #[cfg(not(ossl111))]
             {
                 let _ = vm;
                 Ok(0)
             }

-            #[cfg(ossl110)]
+            #[cfg(ossl111)]
             {
                 let ctx = self.builder();
                 let result = unsafe { sys::SSL_CTX_set_num_tickets(ctx.as_ptr(), value as usize) };
                 if result != 1 {
                     return Err(vm.new_value_error("failed to set num tickets."));
                 }
                 Ok(())
             }
-            #[cfg(not(ossl110))]
+            #[cfg(not(ossl111))]
             {
                 let _ = (value, vm);
                 Ok(())
             }

Also applies to: 1073-1101

common/src/fileutils.rs (1)

475-476: Use crate::os::last_os_error() for reliable Windows error mapping (CRT errno vs GetLastError).

Replace std::io::Error::last_os_error() with crate::os::last_os_error() at these sites to avoid code 0 from CRT-only failures.

-            return Err(std::io::Error::last_os_error());
+            return Err(crate::os::last_os_error());
@@
-            unsafe { libc::close(fd) };
-            return Err(std::io::Error::last_os_error());
+            unsafe { libc::close(fd) };
+            return Err(crate::os::last_os_error());
@@
-        return Err(std::io::Error::last_os_error());
+        return Err(crate::os::last_os_error());
@@
-        return Err(std::io::Error::last_os_error());
+        return Err(crate::os::last_os_error());

Run to catch any remaining instances:

#!/bin/bash
rg -nP --type=rust -C1 '\bstd::io::Error::last_os_error\s*\(' common/src/fileutils.rs

Also applies to: 481-484, 526-527, 531-533

stdlib/src/ssl/ssl_data_31.rs (4)

1-4: Stabilize generator header for reproducible builds.

Drop the wall-clock timestamp and avoid embedding ephemeral source paths like “/tmp/openssl-3.1”. Make the header deterministic (e.g., version/commit only). Also, the path says tools/make_ssl_data_rs.py; please verify the actual script location and make it consistent across files.

---

8-10: Use doc comments on public items.

Convert these explanatory comments to /// so they appear in generated docs for LIBRARY_CODES and ERROR_CODES.

Also applies to: 67-70

---

1768-1770: Make encode_error_key const, take u32, and mask both halves.

Prevents accidental sign issues and allows compile-time evaluation.

-#[inline]
-pub fn encode_error_key(lib: i32, reason: i32) -> u64 {
-    ((lib as u64) << 32) | (reason as u64 & 0xFFFFFFFF)
-}
+#[inline]
+pub const fn encode_error_key(lib: u32, reason: u32) -> u64 {
+    ((lib as u64 & 0xFFFF_FFFF) << 32) | (reason as u64 & 0xFFFF_FFFF)
+}

If call sites pass signed ints, adjust them to u32 (or cast once at the call site).

---

70-1754: Consider cfg-gating to compile only one version’s table.

These maps are large and repeated across versions; compiling only the active OpenSSL mapping can cut compile time and binary size.

Confirm that the top-level selection already gates this; if not, add a cargo feature or cfg guard to pick exactly one ssl_data_* at build time.

stdlib/src/ssl/ssl_data_111.rs (4)

1-4: Deterministic header, and path consistency.

Remove the timestamp, avoid /tmp/… paths, and ensure the generator path string matches the actual script location.

---

8-10: Promote comments to rustdoc.

Switch // to /// above the exported statics so users get inline docs.

Also applies to: 58-61

---

1345-1347: Harden and const-ify encode_error_key.

Mirror the change proposed in the 3.1 file for type safety and const evaluation.

-#[inline]
-pub fn encode_error_key(lib: i32, reason: i32) -> u64 {
-    ((lib as u64) << 32) | (reason as u64 & 0xFFFFFFFF)
-}
+#[inline]
+pub const fn encode_error_key(lib: u32, reason: u32) -> u64 {
+    ((lib as u64 & 0xFFFF_FFFF) << 32) | (reason as u64 & 0xFFFF_FFFF)
+}

---

61-1341: Optional: compile-time gating to avoid duplicate tables.

If only one OpenSSL flavor is used per build, gate the others with cfg/features to save compile time and binary size.

stdlib/src/ssl/ssl_data_300.rs (4)

1-4: Make generation metadata reproducible.

Strip timestamps and non-reproducible source paths; keep only OpenSSL version/commit and generator name.

---

8-10: Prefer rustdoc for public statics.

Change these descriptive // comments to /// above LIBRARY_CODES and ERROR_CODES.

Also applies to: 67-70

---

1757-1759: Const + u32 params for encode_error_key.

Align with the other modules; safer types and const evaluable.

-#[inline]
-pub fn encode_error_key(lib: i32, reason: i32) -> u64 {
-    ((lib as u64) << 32) | (reason as u64 & 0xFFFFFFFF)
-}
+#[inline]
+pub const fn encode_error_key(lib: u32, reason: u32) -> u64 {
+    ((lib as u64 & 0xFFFF_FFFF) << 32) | (reason as u64 & 0xFFFF_FFFF)
+}

---

70-1753: Feature/cfg gating to limit compiled datasets.

If runtime doesn’t need all versions, add cfg selection to compile exactly one dataset.

common/src/fileutils.rs (2)

479-481: Avoid CString allocation for constant "rb".

Since only "rb" is accepted, pass a static C string literal.

-        let mode_cstr = CString::new(mode).unwrap();
-        let fp = unsafe { _fdopen(fd, mode_cstr.as_ptr()) };
+        let fp = unsafe { _fdopen(fd, b"rb\0".as_ptr() as *const libc::c_char) };
@@
-        let mode_cstr = CString::new(mode).unwrap();
-        let fp = unsafe { libc::fdopen(fd, mode_cstr.as_ptr()) };
+        let fp = unsafe { libc::fdopen(fd, b"rb\0".as_ptr() as *const libc::c_char) };

Also applies to: 503-505

---

506-508: Consistent error helper across platforms (optional).

For consistency with this file’s other paths, consider using crate::os::last_os_error() here too.

-            return Err(std::io::Error::last_os_error());
+            return Err(crate::os::last_os_error());

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

common/src/fileutils.rs (4)

504-508: Use crate::os::last_os_error() consistently across the codebase.

While less critical on Unix than Windows, the previous review comment requested consistent use of crate::os::last_os_error() throughout fopen() for uniformity.

Apply this diff:

         if fp.is_null() {
             unsafe { libc::close(fd) };
-            return Err(std::io::Error::last_os_error());
+            return Err(crate::os::last_os_error());
         }

---

471-477: Fix Windows HANDLE leak on _open_osfhandle failure and pass O_BINARY for binary mode.

This issue was previously flagged: If _open_osfhandle returns -1, the raw HANDLE from into_raw_handle() is leaked because ownership was transferred but CloseHandle is not called. Additionally, for binary read mode, O_BINARY should be passed to prevent text-mode translation.

Apply this diff:

-        let fd = unsafe { _open_osfhandle(handle as isize, libc::O_RDONLY) };
+        let fd = unsafe { _open_osfhandle(handle as isize, libc::O_RDONLY | libc::O_BINARY) };
         if fd == -1 {
+            // _open_osfhandle did not take ownership; close the raw HANDLE to avoid a leak.
+            unsafe { windows_sys::Win32::Foundation::CloseHandle(handle as _) };
-            return Err(std::io::Error::last_os_error());
+            return Err(crate::os::last_os_error());
         }

---

481-484: Use crate::os::last_os_error() instead of std::io::Error::last_os_error().

This issue was previously flagged: On Windows, _fdopen sets errno but not GetLastError, making std::io::Error::last_os_error() unreliable. Use crate::os::last_os_error() which bridges this gap.

Apply this diff:

         if fp.is_null() {
             unsafe { libc::close(fd) };
-            return Err(std::io::Error::last_os_error());
+            return Err(crate::os::last_os_error());
         }

---

519-536: Use crate::os::last_os_error() in set_inheritable for Windows CRT/API calls.

This issue was previously flagged: Both get_osfhandle (CRT) and SetHandleInformation (Windows API) error handling should use crate::os::last_os_error() instead of std::io::Error::last_os_error() for reliable errno/GetLastError bridging.

Apply this diff:

         let handle = unsafe { libc::get_osfhandle(fd) };
         if handle == INVALID_HANDLE_VALUE as isize {
-            return Err(std::io::Error::last_os_error());
+            return Err(crate::os::last_os_error());
         }
     
         let flags = if inheritable { HANDLE_FLAG_INHERIT } else { 0 };
         let result = unsafe { SetHandleInformation(handle as HANDLE, HANDLE_FLAG_INHERIT, flags) };
         if result == 0 {
-            return Err(std::io::Error::last_os_error());
+            return Err(crate::os::last_os_error());
         }

scripts/make_ssl_data_rs.py (1)

20-20: Remove unused import.

Apply:

-import sys

stdlib/src/ssl.rs (4)

618-633: Don’t use ex_data index 0; allocate a dedicated index for PySslSocket.

Index 0 isn’t reserved and can collide. Create and use a dedicated index everywhere you store/fetch the PySslSocket pointer.

Apply:

+    // Get or create an ex_data index for storing PySslSocket*
+    fn get_ssl_socket_ex_data_index() -> libc::c_int {
+        use std::sync::LazyLock;
+        static IDX: LazyLock<libc::c_int> = LazyLock::new(|| unsafe {
+            sys::SSL_get_ex_new_index(0, std::ptr::null_mut(), None, None, None)
+        });
+        *IDX
+    }
@@
-            // Get SSL socket from SSL ex_data (stored as PySslSocket pointer)
-            let ssl_socket_ptr = sys::SSL_get_ex_data(ssl_ptr, 0); // Index 0 for SSL socket
+            // Get SSL socket from SSL ex_data (stored as PySslSocket pointer)
+            let ssl_socket_ptr = sys::SSL_get_ex_data(ssl_ptr, get_ssl_socket_ex_data_index());
@@
-                    // Store PyRef pointer (heap-allocated) in ex_data index 0
-                    sys::SSL_set_ex_data(ssl_ptr, 0, &*py_ref as *const _ as *mut _);
+                    // Store PySslSocket pointer in its dedicated ex_data index
+                    sys::SSL_set_ex_data(
+                        ssl_ptr,
+                        get_ssl_socket_ex_data_index(),
+                        &*py_ref as *const _ as *mut _,
+                    );
@@
-                    // Store PyRef pointer (heap-allocated) in ex_data index 0
-                    sys::SSL_set_ex_data(ssl_ptr, 0, &*py_ref as *const _ as *mut _);
+                    // Store PySslSocket pointer in its dedicated ex_data index
+                    sys::SSL_set_ex_data(
+                        ssl_ptr,
+                        get_ssl_socket_ex_data_index(),
+                        &*py_ref as *const _ as *mut _,
+                    );

#!/bin/bash
# Find any remaining uses of ex_data index 0 under ssl.rs
rg -n 'SSL_(get|set)_ex_data\([^,]+,\s*0\b' stdlib/src/ssl.rs -n -C2

Also applies to: 1550-1565, 1695-1697

---

1003-1015: Fix sentinel mapping for minimum_version/maximum_version (map -2/-1 to 0).

Both setters must translate Python sentinels to OpenSSL’s “unset/default” value (0). Current code forces concrete TLS versions.

Apply:

-            let proto_version = match value {
-                -2 => { sys::TLS1_2_VERSION }
-                -1 => { sys::TLS1_3_VERSION }
-                _ => value,
-            };
+            let proto_version = match value {
+                -2 | -1 => 0, // clear limit; let OpenSSL choose min/max
+                _ => value,
+            };

-            let proto_version = match value {
-                -1 => { 0 }
-                -2 => { sys::TLS1_2_VERSION }
-                _ => value,
-            };
+            let proto_version = match value {
+                -1 | -2 => 0, // clear limit; let OpenSSL choose min/max
+                _ => value,
+            };

Also applies to: 1036-1048

---

1058-1072: Gate num_tickets on OpenSSL 1.1.1+, not 1.1.0.

SSL_CTX_{get,set}_num_tickets were added in 1.1.1. Using ossl110 will break on 1.1.0.

Apply:

-            #[cfg(ossl110)]
+            #[cfg(ossl111)]
@@
-            #[cfg(not(ossl110))]
+            #[cfg(not(ossl111))]

and:

-            #[cfg(ossl110)]
+            #[cfg(ossl111)]
@@
-            #[cfg(not(ossl110))]
+            #[cfg(not(ossl111))]

#!/bin/bash
# Check cfg gates around num_tickets use
rg -n 'SSL_CTX_(get|set)_num_tickets|num_tickets\(' stdlib/src/ssl.rs -n -C2

Also applies to: 1074-1101

---

3030-3054: Simplify and correct system error detection to only match ERR_LIB_SYS.

Current logic conflates SSL library errors with system errors and double-checks ERR_LIB_SYS. Restrict to lib == ERR_LIB_SYS.

Apply:

-                // Check both lib code and library string for system errors
-                if lib == sys::ERR_LIB_SYS || library_str == "SSL routines" {
+                // System library error (errno-based)
+                if lib == sys::ERR_LIB_SYS {
                     let reason = sys::ERR_GET_REASON(e.code());
-                    let reason_str = e.reason().unwrap_or("");
-
-                    // Check if it's a system lib error by reason string
-                    if reason_str == "system lib" || lib == sys::ERR_LIB_SYS {
+                    {
                         // errno 2 = ENOENT = FileNotFoundError
                         let exc_type = if reason == 2 {
                             vm.ctx.exceptions.file_not_found_error.to_owned()
                         } else {
                             vm.ctx.exceptions.os_error.to_owned()
                         };
                         let exc = vm.new_exception(exc_type, vec![vm.ctx.new_int(reason).into()]);
                         // Set errno attribute explicitly
                         let _ = exc
                             .as_object()
                             .set_attr("errno", vm.ctx.new_int(reason), vm);
                         return exc;
-                    }
+                    }
                 }

stdlib/src/ssl/ssl_data_31.rs (2)

1-8: Mark generated file to avoid rustfmt/clippy churn.

Add file-scoped attributes so CI isn’t noisy on unreadable numerals and we skip formatting of generated tables.

Apply:

-// File generated by tools/make_ssl_data_rs.py
+// File generated by tools/make_ssl_data_rs.py
+#![allow(clippy::unreadable_literal)]
+#![rustfmt::skip]
 // Generated on 2025-10-29T07:17:16.621749+00:00
 // Source: OpenSSL from /tmp/openssl-3.1
 // spell-checker: disable

---

1766-1770: Avoid helper duplication across ssl_data_ modules.*

encode_error_key is identical in each versioned file. Consider moving it to a shared ssl module and reusing it from here to reduce duplication.

stdlib/src/ssl/ssl_data_111.rs (2)

1-8: Silence clippy on large numerals and skip formatting for generated tables.

Same rationale as ssl_data_31.rs.

Apply:

-// File generated by tools/make_ssl_data_rs.py
+// File generated by tools/make_ssl_data_rs.py
+#![allow(clippy::unreadable_literal)]
+#![rustfmt::skip]
 // Generated on 2025-10-29T07:17:23.692784+00:00
 // Source: OpenSSL from /tmp/openssl-1.1.1
 // spell-checker: disable

---

1343-1347: Deduplicate encode_error_key to cert.rs or a shared utilities module.

The function is identically defined in stdlib/src/ssl/ssl_data_31.rs:1768, ssl_data_111.rs:1345, and ssl_data_300.rs:1757. Move to a shared location and re-export from versioned files. Note: stdlib/src/ssl/mod.rs does not exist; consider cert.rs as the shared location or create a dedicated utilities file. After deduplication, manually verify clippy runs without warnings.

stdlib/src/ssl/ssl_data_300.rs (1)

1-5: Optional: make generated file more stable and lint-quiet.

To reduce churn and lints:

• Drop or gate the timestamp line.
• Add allow attributes at top:

+#![allow(clippy::all)]
+#![allow(dead_code)]

scripts/make_ssl_data_rs.py (3)

127-129: Sorting doc vs behavior mismatch — pick one.

Comment says “sort by libname, numeric error code” but code sorts by reason then number. Prefer lib then number for stable grouping.

Apply one of:

• Fix code:

-    args.reasons = sorted(reasons, key=operator.itemgetter(0, 3))
+    # sort by libname then numeric error code
+    args.reasons = sorted(reasons, key=operator.itemgetter(1, 3))

• Or update the comment to reflect actual sort by reason.

---

90-104: Protect against duplicate (lib, reason) keys when merging openssl.txt and openssl.ec.

Duplicate keys produce duplicate entries in the phf_map literal. Dedup by last-wins for deterministic output.

Apply:

 def gen_error_codes_rust(args):
     """Generate Rust phf map for error codes"""
     yield "// Maps encoded (lib, reason) -> error mnemonic"
     yield '// Example: encode_error_key(20, 134) -> "CERTIFICATE_VERIFY_FAILED"'
     yield "// Key encoding: (lib << 32) | reason"
     yield "pub static ERROR_CODES: phf::Map<u64, &'static str> = phf_map! {"
-    for reason, libname, errname, num in args.reasons:
-        if libname not in args.lib2errnum:
-            continue
-        lib_num = args.lib2errnum[libname]
-        # Encode (lib, reason) as single u64
-        key = (lib_num << 32) | num
-        yield f'    {key}u64 => "{errname}",'
+    entries = {}
+    for reason, libname, errname, num in args.reasons:
+        if libname not in args.lib2errnum:
+            continue
+        lib_num = args.lib2errnum[libname]
+        key = (lib_num << 32) | num
+        entries[key] = errname  # last wins
+    for key in sorted(entries.keys()):
+        yield f'    {key}u64 => "{entries[key]}",'
     yield "};"
     yield ""

---

131-138: Fix header path and consider dropping timestamp for reproducibility.

• Change “tools/make_ssl_data_rs.py” to “scripts/make_ssl_data_rs.py”.
• Optionally gate timestamp with an env var to reduce VCS noise.

Apply:

-        "// File generated by tools/make_ssl_data_rs.py",
+        "// File generated by scripts/make_ssl_data_rs.py",
-        f"// Generated on {datetime.datetime.now(datetime.timezone.utc).isoformat()}",
+        f"// Generated on {datetime.datetime.now(datetime.timezone.utc).isoformat()}",

Or:

-        f"// Generated on {datetime.datetime.now(datetime.timezone.utc).isoformat()}",
+        f"// Generated on {datetime.datetime.now(datetime.timezone.utc).isoformat()}",
+        // Set NO_TIMESTAMP=1 to omit the previous line when reproducibility matters.

stdlib/src/ssl.rs (1)

1511-1532: BIO mode: good change to drive I/O through Read/Write; minor nit on error kinds.

Using WouldBlock for empty BIO is correct. For write errors, prefer a specific ErrorKind.

Apply:

-                if nbytes < 0 {
-                    return Err(std::io::Error::other("BIO write failed"));
-                }
+                if nbytes < 0 {
+                    return Err(std::io::Error::new(std::io::ErrorKind::Other, "BIO write failed"));
+                }

Also applies to: 1637-1658, 1829-1878

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

stdlib/src/ssl/ssl_data_31.rs (1)

1-3: Fix generator header path; add “generated” preamble and clippy allowances via generator.

Header says tools/make_ssl_data_rs.py, but the generator lives under scripts/. Also add a clear “DO NOT EDIT” line and a minimal clippy allow to avoid unreadable-literal noise in generated constants.

Apply in scripts/make_ssl_data_rs.py (not here):

-    lines = [
-        "// File generated by tools/make_ssl_data_rs.py",
+    lines = [
+        "// Code generated by scripts/make_ssl_data_rs.py; DO NOT EDIT.",
+        "// File generated by scripts/make_ssl_data_rs.py",
         f"// Generated on {datetime.datetime.now(datetime.timezone.utc).isoformat()}",
         f"// Source: OpenSSL from {args.srcdir}",
         "// spell-checker: disable",
+        "#![allow(clippy::unreadable_literal)]",
         "",
         "use phf::phf_map;",
         "",
     ]

stdlib/src/ssl/ssl_data_111.rs (1)

1-3: Same header-path issue; add preamble and clippy allow via generator.

Update generator to emit scripts/make_ssl_data_rs.py and add DO NOT EDIT + clippy allow as in the 3.1 file.

See proposed diff in scripts/make_ssl_data_rs.py from the other comment.

common/src/fileutils.rs (2)

519-536: Windows: use crate::os::last_os_error() in set_inheritable for consistent errno/GetLastError handling.

     let handle = unsafe { libc::get_osfhandle(fd) };
     if handle == INVALID_HANDLE_VALUE as isize {
-        return Err(std::io::Error::last_os_error());
+        return Err(crate::os::last_os_error());
     }
@@
     let result = unsafe { SetHandleInformation(handle as HANDLE, HANDLE_FLAG_INHERIT, flags) };
     if result == 0 {
-        return Err(std::io::Error::last_os_error());
+        return Err(crate::os::last_os_error());
     }

---

471-491: Windows: fix HANDLE leak on _open_osfhandle failure; open in binary; use unified last_os_error().

• If _open_osfhandle returns -1, the raw HANDLE from into_raw_handle() leaks; close it.
• Use O_BINARY to avoid text-mode translation.
• Prefer crate::os::last_os_error() (bridges errno/GetLastError).

Apply:

@@
-        use std::os::windows::io::IntoRawHandle;
+        use std::os::windows::io::IntoRawHandle;
+        use windows_sys::Win32::Foundation::CloseHandle;
@@
-        let fd = unsafe { _open_osfhandle(handle as isize, libc::O_RDONLY) };
+        let fd = unsafe { _open_osfhandle(handle as isize, libc::O_RDONLY | libc::O_BINARY) };
         if fd == -1 {
-            return Err(std::io::Error::last_os_error());
+            // _open_osfhandle did not take ownership; close raw HANDLE to avoid leak.
+            unsafe { CloseHandle(handle as _) };
+            return Err(crate::os::last_os_error());
         }
@@
         let fp = unsafe { _fdopen(fd, mode_cstr.as_ptr()) };
         if fp.is_null() {
-            unsafe { libc::close(fd) };
-            return Err(std::io::Error::last_os_error());
+            unsafe { libc::close(fd) };
+            return Err(crate::os::last_os_error());
         }

scripts/make_ssl_data_rs.py (3)

124-129: Don’t fail if crypto/err/openssl.ec is absent (older trees).

-    reasons = []
-    reasons.extend(parse_openssl_error_text(args))
-    reasons.extend(parse_extra_reasons(args))
+    reasons = []
+    reasons.extend(parse_openssl_error_text(args))
+    if os.path.isfile(args.errcodes):
+        reasons.extend(parse_extra_reasons(args))

---

131-138: Emit correct header path, DO NOT EDIT banner, and clippy allow in generated files.

Keeps headers accurate and reduces lint noise on huge constants.

-    lines = [
-        "// File generated by tools/make_ssl_data_rs.py",
+    lines = [
+        "// Code generated by scripts/make_ssl_data_rs.py; DO NOT EDIT.",
+        "// File generated by scripts/make_ssl_data_rs.py",
         f"// Generated on {datetime.datetime.now(datetime.timezone.utc).isoformat()}",
         f"// Source: OpenSSL from {args.srcdir}",
         "// spell-checker: disable",
+        "#![allow(clippy::unreadable_literal)]",
         "",
         "use phf::phf_map;",
         "",
     ]

---

20-20: Remove unused import.

-import sys

stdlib/src/ssl.rs (3)

618-619: CRITICAL: Still using hardcoded ex_data index 0 for PySslSocket.

This critical issue from the previous review remains unfixed. Index 0 is not guaranteed to be yours; using it risks collisions with other code or undefined behavior. You must allocate a dedicated index via SSL_get_ex_new_index.

Apply the fix from the previous review:

+    // Get or create an ex_data index for storing PySslSocket*
+    fn get_ssl_socket_ex_data_index() -> libc::c_int {
+        use std::sync::LazyLock;
+        static IDX: LazyLock<libc::c_int> = LazyLock::new(|| unsafe {
+            sys::SSL_get_ex_new_index(0, std::ptr::null_mut(), None, None, None)
+        });
+        *IDX
+    }

Then replace all three occurrences:

-            let ssl_socket_ptr = sys::SSL_get_ex_data(ssl_ptr, 0);
+            let ssl_socket_ptr = sys::SSL_get_ex_data(ssl_ptr, get_ssl_socket_ex_data_index());

-                    sys::SSL_set_ex_data(ssl_ptr, 0, &*py_ref as *const _ as *mut _);
+                    sys::SSL_set_ex_data(ssl_ptr, get_ssl_socket_ex_data_index(), &*py_ref as *const _ as *mut _);

Also applies to: 1562-1563, 1695-1696

---

1003-1015: CRITICAL: Sentinel handling for protocol versions remains incorrect despite being marked "addressed".

The previous review clearly identified this issue with web search verification, yet the problematic code is unchanged:

In set_minimum_version (lines 1004-1015):

• -2 (PY_PROTO_MINIMUM_SUPPORTED) still maps to TLS1_2_VERSION instead of 0
• -1 (PY_PROTO_MAXIMUM_SUPPORTED) still maps to TLS1_3_VERSION instead of 0

In set_maximum_version (lines 1037-1048):

• -2 still maps to TLS1_2_VERSION instead of 0

Both sentinels must map to 0 (OpenSSL's "unset/default") to maintain symmetry with the getters and allow OpenSSL to use its minimum/maximum supported versions.

Apply the previously suggested fix:

-            let proto_version = match value {
-                -2 => {
-                    // PY_PROTO_MINIMUM_SUPPORTED -> use minimum available (TLS 1.2)
-                    sys::TLS1_2_VERSION
-                }
-                -1 => {
-                    // PY_PROTO_MAXIMUM_SUPPORTED -> use maximum available
-                    // For max on min_proto_version, we use the newest available
-                    sys::TLS1_3_VERSION
-                }
-                _ => value,
-            };
+            // -2 (minimum supported) -> 0 to clear limit; -1 (maximum supported) is not meaningful here,
+            // but mirror CPython by treating it as 0 as well.
+            let proto_version = match value {
+                -2 | -1 => 0,
+                _ => value,
+            };

-            let proto_version = match value {
-                -1 => {
-                    // PY_PROTO_MAXIMUM_SUPPORTED -> use 0 for OpenSSL (means no limit)
-                    0
-                }
-                -2 => {
-                    // PY_PROTO_MINIMUM_SUPPORTED -> use minimum available (TLS 1.2)
-                    sys::TLS1_2_VERSION
-                }
-                _ => value,
-            };
+            // -1 (maximum supported) -> 0; -2 (minimum supported) is not meaningful here,
+            // but mirror CPython by treating it as 0 as well.
+            let proto_version = match value {
+                -1 | -2 => 0,
+                _ => value,
+            };

Also applies to: 1036-1048

---

1058-1072: CRITICAL: cfg gates for num_tickets still use ossl110 instead of ossl111.

This critical issue from the previous review remains unfixed. SSL_CTX_get_num_tickets and SSL_CTX_set_num_tickets were added in OpenSSL 1.1.1, not 1.1.0. Using #[cfg(ossl110)] will cause build failures on OpenSSL 1.1.0.

Change the cfg attributes at all four locations:

-            #[cfg(ossl110)]
+            #[cfg(ossl111)]
             {
                 let ctx = self.ctx();
                 let num = unsafe { sys::SSL_CTX_get_num_tickets(ctx.as_ptr()) };
                 Ok(num)
             }
-            #[cfg(not(ossl110))]
+            #[cfg(not(ossl111))]
             {

-            #[cfg(ossl110)]
+            #[cfg(ossl111)]
             {
                 let ctx = self.builder();
                 let result = unsafe { sys::SSL_CTX_set_num_tickets(ctx.as_ptr(), value as usize) };
                 if result != 1 {
                     return Err(vm.new_value_error("failed to set num tickets."));
                 }
                 Ok(())
             }
-            #[cfg(not(ossl110))]
+            #[cfg(not(ossl111))]
             {

Also applies to: 1074-1101

stdlib/src/socket.rs (1)

1254-1266: Align recvfrom_into semantics with recv_into (clamp instead of error when nbytes > len(buf)).

recv_into now clamps nbytes to the buffer length, but recvfrom_into errors if nbytes exceeds buffer size. CPython semantics read “at most nbytes,” so clamping is preferable and consistent.

Apply this refactor:

@@
-            let buf = match nbytes {
-                OptionalArg::Present(i) => {
-                    let i = i.to_usize().ok_or_else(|| {
-                        vm.new_value_error("negative buffersize in recvfrom_into")
-                    })?;
-                    buf.get_mut(..i).ok_or_else(|| {
-                        vm.new_value_error("nbytes is greater than the length of the buffer")
-                    })?
-                }
-                OptionalArg::Missing => buf,
-            };
+            let read_len = match nbytes {
+                OptionalArg::Present(i) => {
+                    let i = i.to_usize().ok_or_else(|| {
+                        vm.new_value_error("negative buffersize in recvfrom_into")
+                    })?;
+                    i.min(buf.len())
+                }
+                OptionalArg::Missing => buf.len(),
+            };
+            let buf = &mut buf[..read_len];

stdlib/src/ssl/ssl_data_31.rs (1)

1768-1770: Make encode_error_key a const fn and avoid sign-ext casting.

Small polish and clippy-friendly style.

Apply in scripts/make_ssl_data_rs.py:

-            "pub fn encode_error_key(lib: i32, reason: i32) -> u64 {",
-            "    ((lib as u64) << 32) | (reason as u64 & 0xFFFFFFFF)",
+            "pub const fn encode_error_key(lib: i32, reason: i32) -> u64 {",
+            "    ((lib as u64) << 32) | u64::from(reason as u32)",
             "}",

stdlib/src/ssl/ssl_data_111.rs (1)

1345-1347: Make encode_error_key const and use u64::from for masking.

Covered by the generator change suggested earlier.

stdlib/src/ssl/ssl_data_300.rs (1)

1757-1759: Promote encode_error_key to const and use u64::from(reason as u32).

Keeps consistency across versions and improves clarity.

Address centrally in scripts/make_ssl_data_rs.py as shown in earlier diffs.

common/src/fileutils.rs (1)

449-456: Optional: accept “r”/“rt” as aliases for “rb”.

Quality-of-life improvement; keeps strictness while easing caller usage.

-    if mode != "rb" {
+    if mode != "rb" && mode != "r" && mode != "rt" {
         return Err(std::io::Error::new(
             std::io::ErrorKind::InvalidInput,
             format!("unsupported mode: {}", mode),
         ));
     }

scripts/make_ssl_data_rs.py (2)

145-151: Generate const fn and clippy‑friendly mask in helper.

Keeps all versioned modules consistent and cleaner.

-            "pub fn encode_error_key(lib: i32, reason: i32) -> u64 {",
-            "    ((lib as u64) << 32) | (reason as u64 & 0xFFFFFFFF)",
+            "pub const fn encode_error_key(lib: i32, reason: i32) -> u64 {",
+            "    ((lib as u64) << 32) | u64::from(reason as u32)",
             "}",

---

77-86: Minor: comment says “keep the last one if duplicates” but code picks last by lexicographic libname.

Either clarify the comment or preserve input order so later defines in err.h win. Low impact.

-    # Deduplicate: keep the last one if there are duplicates
-    seen = {}
-    for libname in sorted(args.lib2errnum):
+    # Deduplicate: keep the last one by input order if there are duplicates
+    seen = {}
+    for libname in args.lib2errnum:  # relies on dict insertion order (Py3.7+)
         lib_num = args.lib2errnum[libname]
         seen[lib_num] = libname

stdlib/src/ssl.rs (2)

1829-1939: BIO pathway logic appears improved, but verify BioStream error handling.

The removal of SSL_set_bio (line 1635 comment) and implementation of Read/Write for BioStream addresses the previous critical issue about conflicting BIO setup.

However, the BioStream Read implementation (lines 1836-1856) returns WouldBlock for all negative BIO_read results without checking whether it's actually a retry condition. This might mask real errors.

Consider checking BIO_should_retry to distinguish between transient and permanent errors:

             let nbytes = sys::BIO_read(
                 self.inbio.bio,
                 buf.as_mut_ptr() as *mut _,
                 buf.len().min(i32::MAX as usize) as i32,
             );
             if nbytes < 0 {
-                // BIO_read returns -1 on error or when no data is available
-                // Check if it's a retry condition (WANT_READ)
-                Err(std::io::Error::new(
-                    std::io::ErrorKind::WouldBlock,
-                    "BIO has no data available",
-                ))
+                if sys::BIO_should_retry(self.inbio.bio) != 0 {
+                    Err(std::io::Error::new(
+                        std::io::ErrorKind::WouldBlock,
+                        "BIO has no data available",
+                    ))
+                } else {
+                    Err(std::io::Error::other("BIO_read failed"))
+                }
             } else {
                 Ok(nbytes as usize)
             }

---

1354-1417: SNI callback setter is safe given ownership structure, but consider adding safety comment.

The code sets self as a raw pointer for SSL_CTX_set_tlsext_servername_arg (line 1392). This is safe because:

• PySslSocket holds PyRef<PySslContext>, keeping the context alive
• The pointer is only dereferenced during the SNI callback (handshake)
• The context cannot be moved while the SSL_CTX exists

Consider adding a safety comment at line 1392:

                     sys::SSL_CTX_set_tlsext_servername_arg(
                         self.ctx().as_ptr(),
+                        // SAFETY: self pointer is valid as long as this PySslContext exists.
+                        // PySslSocket holds a PyRef<PySslContext>, ensuring it outlives any SSL connection.
                         self as *const _ as *mut _,
                     );

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro