Skip to content

more ssl impl #6228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Oct 29, 2025
Merged

more ssl impl #6228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions Lib/test/test_httplib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1801,8 +1801,6 @@ def test_local_good_hostname(self):
self.addCleanup(resp.close)
self.assertEqual(resp.status, 404)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_local_bad_hostname(self):
# The (valid) cert doesn't validate the HTTP hostname
import ssl
Expand Down
21 changes: 0 additions & 21 deletions Lib/test/test_ssl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,6 @@ def test_refcycle(self):
del ss
self.assertEqual(wr(), None)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_wrapped_unconnected(self):
# Methods on an unconnected SSLSocket propagate the original
# OSError raise by the underlying socket object.
Expand Down Expand Up @@ -1400,7 +1399,6 @@ def test_load_dh_params(self):
with self.assertRaises(ssl.SSLError) as cm:
ctx.load_dh_params(CERTFILE)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_session_stats(self):
for proto in {ssl.PROTOCOL_TLS_CLIENT, ssl.PROTOCOL_TLS_SERVER}:
ctx = ssl.SSLContext(proto)
Expand Down Expand Up @@ -1461,7 +1459,6 @@ def dummycallback(sock, servername, ctx, cycle=ctx):
gc.collect()
self.assertIs(wr(), None)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_cert_store_stats(self):
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
self.assertEqual(ctx.cert_store_stats(),
Expand Down Expand Up @@ -1521,7 +1518,6 @@ def test_load_default_certs(self):
self.assertRaises(TypeError, ctx.load_default_certs, 'SERVER_AUTH')

@unittest.skipIf(sys.platform == "win32", "not-Windows specific")
@unittest.expectedFailure # TODO: RUSTPYTHON
def test_load_default_certs_env(self):
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
with os_helper.EnvironmentVarGuard() as env:
Expand Down Expand Up @@ -1673,7 +1669,6 @@ def test_context_client_server(self):
self.assertFalse(ctx.check_hostname)
self.assertEqual(ctx.verify_mode, ssl.CERT_NONE)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_context_custom_class(self):
class MySSLSocket(ssl.SSLSocket):
pass
Expand All @@ -1690,7 +1685,6 @@ class MySSLObject(ssl.SSLObject):
obj = ctx.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), server_side=True)
self.assertIsInstance(obj, MySSLObject)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_num_tickest(self):
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
self.assertEqual(ctx.num_tickets, 2)
Expand Down Expand Up @@ -1721,7 +1715,6 @@ def test_str(self):
self.assertEqual(str(e), "foo")
self.assertEqual(e.errno, 1)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_lib_reason(self):
# Test the library and reason attributes
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
Expand Down Expand Up @@ -1899,7 +1892,6 @@ def setUp(self):
self.enterContext(server)
self.server_addr = (HOST, server.port)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_connect(self):
with test_wrap_socket(socket.socket(socket.AF_INET),
cert_reqs=ssl.CERT_NONE) as s:
Expand Down Expand Up @@ -1966,7 +1958,6 @@ def test_non_blocking_connect_ex(self):
# SSL established
self.assertTrue(s.getpeercert())

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_connect_with_context(self):
# Same as test_connect, but with a separately created context
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
Expand Down Expand Up @@ -2093,7 +2084,6 @@ def test_non_blocking_handshake(self):
def test_get_server_certificate(self):
_test_get_server_certificate(self, *self.server_addr, cert=SIGNING_CA)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_get_server_certificate_sni(self):
host, port = self.server_addr
server_names = []
Expand All @@ -2120,7 +2110,6 @@ def test_get_server_certificate_fail(self):
# independent test method
_test_get_server_certificate_fail(self, *self.server_addr)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_get_server_certificate_timeout(self):
def servername_cb(ssl_sock, server_name, initial_context):
time.sleep(0.2)
Expand Down Expand Up @@ -2156,7 +2145,6 @@ def test_get_ca_certs_capath(self):
self.assertTrue(cert)
self.assertEqual(len(ctx.get_ca_certs()), 1)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_context_setget(self):
# Check that the context of a connected socket can be replaced.
ctx1 = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
Expand Down Expand Up @@ -3041,7 +3029,6 @@ def test_crl_check(self):
cert = s.getpeercert()
self.assertTrue(cert, "Can't get peer certificate.")

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_check_hostname(self):
if support.verbose:
sys.stdout.write("\n")
Expand Down Expand Up @@ -3182,7 +3169,6 @@ def test_dual_rsa_ecc(self):
cipher = s.cipher()[0].split('-')
self.assertTrue(cipher[:2], ('ECDHE', 'ECDSA'))

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_check_hostname_idn(self, warnings_filters=True):
if support.verbose:
sys.stdout.write("\n")
Expand Down Expand Up @@ -3370,7 +3356,6 @@ def connector():
finally:
t.join()

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_ssl_cert_verify_error(self):
if support.verbose:
sys.stdout.write("\n")
Expand Down Expand Up @@ -3477,7 +3462,6 @@ def test_protocol_tlsv1_1(self):
try_protocol_combo(ssl.PROTOCOL_TLSv1_2, ssl.PROTOCOL_TLSv1_1, False)

@requires_tls_version('TLSv1_2')
@unittest.expectedFailure # TODO: RUSTPYTHON
def test_protocol_tlsv1_2(self):
"""Connecting to a TLSv1.2 server with various client options.
Testing against older TLS versions."""
Expand Down Expand Up @@ -3924,7 +3908,6 @@ def test_do_handshake_enotconn(self):
sock.do_handshake()
self.assertEqual(cm.exception.errno, errno.ENOTCONN)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_no_shared_ciphers(self):
client_context, server_context, hostname = testing_context()
# OpenSSL enables all TLS 1.3 ciphers, enforce TLS 1.2 for test
Expand Down Expand Up @@ -4156,7 +4139,6 @@ def test_no_legacy_server_connect(self):
chatty=True, connectionchatty=True,
sni_name=hostname)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_dh_params(self):
# Check we can get a connection with ephemeral finite-field
# Diffie-Hellman (if supported).
Expand Down Expand Up @@ -4755,7 +4737,6 @@ def test_pha_optional(self):
s.write(b'HASCERT')
self.assertEqual(s.recv(1024), b'TRUE\n')

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_pha_optional_nocert(self):
if support.verbose:
sys.stdout.write("\n")
Expand Down Expand Up @@ -4795,7 +4776,6 @@ def test_pha_no_pha_client(self):
s.write(b'PHA')
self.assertIn(b'extension not received', s.recv(1024))

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_pha_no_pha_server(self):
# server doesn't have PHA enabled, cert is requested in handshake
client_context, server_context, hostname = testing_context()
Expand All @@ -4816,7 +4796,6 @@ def test_pha_no_pha_server(self):
s.write(b'HASCERT')
self.assertEqual(s.recv(1024), b'TRUE\n')

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_pha_not_tls13(self):
# TLS 1.2
client_context, server_context, hostname = testing_context()
Expand Down
2 changes: 0 additions & 2 deletions Lib/test/test_urllib2_localnet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,6 @@ def test_https_with_cadefault(self):
self.urlopen("https://localhost:%s/bizarre" % handler.port,
cadefault=True)

# TODO: RUSTPYTHON
@unittest.expectedFailure
@unittest.skipIf(os.name == "nt", "TODO: RUSTPYTHON, ValueError: illegal environment variable name")
def test_https_sni(self):
if ssl is None:
Expand Down
96 changes: 96 additions & 0 deletions common/src/fileutils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,3 +438,99 @@ pub mod windows {
}
}
}

// _Py_fopen_obj in cpython (Python/fileutils.c:1757-1835)
// Open a file using std::fs::File and convert to FILE*
// Automatically handles path encoding and EINTR retries
pub fn fopen(path: &std::path::Path, mode: &str) -> std::io::Result<*mut libc::FILE> {
use std::ffi::CString;
use std::fs::File;

// Currently only supports read mode
// Can be extended to support "wb", "w+b", etc. if needed
if mode != "rb" {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidInput,
format!("unsupported mode: {}", mode),
));
}

// Open file using std::fs::File (handles path encoding and EINTR automatically)
let file = File::open(path)?;

#[cfg(windows)]
{
use std::os::windows::io::IntoRawHandle;

// Declare Windows CRT functions
unsafe extern "C" {
fn _open_osfhandle(handle: isize, flags: libc::c_int) -> libc::c_int;
fn _fdopen(fd: libc::c_int, mode: *const libc::c_char) -> *mut libc::FILE;
}

// Convert File handle to CRT file descriptor
let handle = file.into_raw_handle();
let fd = unsafe { _open_osfhandle(handle as isize, libc::O_RDONLY) };
if fd == -1 {
return Err(std::io::Error::last_os_error());
}

Comment on lines +471 to +477
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 29, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix Windows HANDLE leak on _open_osfhandle failure; pass O_BINARY for "rb".

  • If _open_osfhandle returns -1, the raw HANDLE from into_raw_handle() is leaked. Close it with CloseHandle.
  • For binary read mode, pass _O_BINARY to prevent text-mode translation.

Apply:

-        let fd = unsafe { _open_osfhandle(handle as isize, libc::O_RDONLY) };
-        if fd == -1 {
-            return Err(std::io::Error::last_os_error());
-        }
+        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(crate::os::last_os_error());
+        }

(Optional: also add _O_NOINHERIT if available; not required since you call SetHandleInformation later.)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In common/src/fileutils.rs around lines 471 to 477, if _open_osfhandle returns
-1 the raw HANDLE obtained from file.into_raw_handle() is leaked and the file is
opened in text mode; fix by calling CloseHandle(handle) before returning the
error to avoid the HANDLE leak, and pass _O_BINARY (and optionally _O_NOINHERIT
if desired) combined with libc::O_RDONLY to _open_osfhandle so the descriptor is
opened in binary read mode (and non-inheritable if used).

// Convert fd to FILE*
let mode_cstr = CString::new(mode).unwrap();
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());
}

// Set non-inheritable (Windows needs this explicitly)
if let Err(e) = set_inheritable(fd, false) {
unsafe { libc::fclose(fp) };
return Err(e);
}

Ok(fp)
}

#[cfg(not(windows))]
{
use std::os::fd::IntoRawFd;

// Convert File to raw fd
let fd = file.into_raw_fd();

// Convert fd to FILE*
let mode_cstr = CString::new(mode).unwrap();
let fp = unsafe { libc::fdopen(fd, mode_cstr.as_ptr()) };
if fp.is_null() {
unsafe { libc::close(fd) };
return Err(std::io::Error::last_os_error());
}

// Unix: O_CLOEXEC is already set by File::open, so non-inheritable is automatic
Ok(fp)
}
}

// set_inheritable in cpython (Python/fileutils.c:1443-1570)
// Set the inheritable flag of the specified file descriptor
// Only used on Windows; Unix automatically sets O_CLOEXEC
#[cfg(windows)]
fn set_inheritable(fd: libc::c_int, inheritable: bool) -> std::io::Result<()> {
use windows_sys::Win32::Foundation::{
HANDLE, HANDLE_FLAG_INHERIT, INVALID_HANDLE_VALUE, SetHandleInformation,
};

let handle = unsafe { libc::get_osfhandle(fd) };
if handle == INVALID_HANDLE_VALUE as isize {
return Err(std::io::Error::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());
}

Ok(())
}
Loading
Loading