From 14c262ab2b1f75d8fa66faec84014d91fe09ac62 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 29 Sep 2026 09:25:26 +0800 Subject: [PATCH] fix(fetch): accept close-delimited HTTPS responses Enable the curl TLS compatibility path for complete HTTP responses whose peer closes without TLS close_notify. Pin the corresponding curl bindings revision and add local TLS fixtures for close-delimited bodies, complete framed responses and truncated body rejection. The fixture tolerates a socket already closed by the client. Validation: all branch checks passed. Summary [ 144.422s] 18928 tests run: 18928 passed (9 slow, 2 flaky), 16 skipped --- Cargo.lock | 4 +- Cargo.toml | 4 +- moli-curl/Cargo.toml | 2 +- moli-curl/tests/tls_eof.rs | 301 +++++++++++++++++++++++++++++++++++++ 4 files changed, 306 insertions(+), 5 deletions(-) create mode 100644 moli-curl/tests/tls_eof.rs diff --git a/Cargo.lock b/Cargo.lock index 85928ba0ca..c509be6575 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -845,7 +845,7 @@ dependencies = [ [[package]] name = "curl" version = "0.4.49" -source = "git+https://github.com/lexmount/curl-rust?rev=a0ea59f5ca1b1e4bc8627a28c29463ac96aeb758#a0ea59f5ca1b1e4bc8627a28c29463ac96aeb758" +source = "git+https://github.com/lexmount/curl-rust?rev=a8c770e9a1765509cacfb852b2720cb3c672ce17#a8c770e9a1765509cacfb852b2720cb3c672ce17" dependencies = [ "curl-sys", "libc", @@ -859,7 +859,7 @@ dependencies = [ [[package]] name = "curl-sys" version = "0.4.87+curl-8.19.0" -source = "git+https://github.com/lexmount/curl-rust?rev=a0ea59f5ca1b1e4bc8627a28c29463ac96aeb758#a0ea59f5ca1b1e4bc8627a28c29463ac96aeb758" +source = "git+https://github.com/lexmount/curl-rust?rev=a8c770e9a1765509cacfb852b2720cb3c672ce17#a8c770e9a1765509cacfb852b2720cb3c672ce17" dependencies = [ "brotlic-sys", "cc", diff --git a/Cargo.toml b/Cargo.toml index bedd190767..6ca11a893e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,8 +98,8 @@ license = "MIT OR Apache-2.0" [patch.crates-io] xml5ever = { path = "vendor/xml5ever-0.39.0" } -curl = { git = "https://github.com/lexmount/curl-rust", rev = "a0ea59f5ca1b1e4bc8627a28c29463ac96aeb758" } -curl-sys = { git = "https://github.com/lexmount/curl-rust", rev = "a0ea59f5ca1b1e4bc8627a28c29463ac96aeb758" } +curl = { git = "https://github.com/lexmount/curl-rust", rev = "a8c770e9a1765509cacfb852b2720cb3c672ce17" } +curl-sys = { git = "https://github.com/lexmount/curl-rust", rev = "a8c770e9a1765509cacfb852b2720cb3c672ce17" } cookie = { git = "https://github.com/ldm0/cookie-rs", branch = "priority" } v8 = { path = "vendor/v8-152.2.0" } deno_v8 = { path = "vendor/deno_v8-0.3.0" } diff --git a/moli-curl/Cargo.toml b/moli-curl/Cargo.toml index 3f02f95d01..c2eed2e430 100644 --- a/moli-curl/Cargo.toml +++ b/moli-curl/Cargo.toml @@ -9,7 +9,7 @@ anyhow = "1.0.100" cidr = "0.3.2" crossbeam-channel = "0.5.15" curl = { version = "0.4.49", default-features = false, features = ["brotli", "http2", "poll_7_68_0", "ssl", "static-curl", "websocket"] } -curl-sys = { version = "0.4.87", default-features = false } +curl-sys = { version = "0.4.87", default-features = false, features = ["allow-missing-close-notify"] } moli-header-field = { path = "../moli-header-field" } moli-dns-resolver = { path = "../moli-dns-resolver" } openssl-sys = { version = "0.9", features = ["aws-lc"] } diff --git a/moli-curl/tests/tls_eof.rs b/moli-curl/tests/tls_eof.rs new file mode 100644 index 0000000000..74fcc86b9d --- /dev/null +++ b/moli-curl/tests/tls_eof.rs @@ -0,0 +1,301 @@ +//! Browser-compatible TLS EOF handling must leave HTTP framing and TLS errors intact. + +use std::{ + fs, + io::{Read, Write}, + net::{Shutdown, TcpListener}, + sync::Arc, + thread, + time::{Duration, Instant}, +}; + +use anyhow::{Result, bail}; +use curl::easy::{Easy2, Handler, HttpVersion, WriteError}; +use moli_curl::CurlTlsConfig; +use rustls::{ServerConfig, ServerConnection, StreamOwned, pki_types::PrivatePkcs8KeyDer}; + +const DEADLINE: Duration = Duration::from_secs(5); + +#[derive(Clone, Copy, Debug)] +enum Ending { + CloseNotify, + TcpEof, + InvalidRecord, +} + +#[derive(Clone, Copy)] +enum Protocol { + Http1, + Http2, +} + +#[derive(Default)] +struct Body(Vec); + +impl Handler for Body { + fn write(&mut self, bytes: &[u8]) -> Result { + self.0.extend_from_slice(bytes); + Ok(bytes.len()) + } +} + +fn exchange( + response: &'static [u8], + ending: Ending, + trust_server: bool, +) -> Result<(Result<(), curl::Error>, Vec)> { + exchange_protocol(response, ending, trust_server, Protocol::Http1) +} + +fn exchange_protocol( + response: &'static [u8], + ending: Ending, + trust_server: bool, + protocol: Protocol, +) -> Result<(Result<(), curl::Error>, Vec)> { + let cert = rcgen::generate_simple_self_signed(vec!["127.0.0.1".to_owned()])?; + let mut config = ServerConfig::builder() + .with_no_client_auth() + .with_single_cert( + vec![cert.cert.der().clone()], + PrivatePkcs8KeyDer::from(cert.key_pair.serialize_der()).into(), + )?; + config.alpn_protocols = vec![match protocol { + Protocol::Http1 => b"http/1.1".to_vec(), + Protocol::Http2 => b"h2".to_vec(), + }]; + let config = Arc::new(config); + let fixtures = tempfile::tempdir()?; + let ca = fixtures.path().join("ca.pem"); + fs::write(&ca, cert.cert.pem())?; + + let listener = TcpListener::bind("127.0.0.1:0")?; + let address = listener.local_addr()?; + listener.set_nonblocking(true)?; + let server = thread::spawn(move || -> Result<()> { + let deadline = Instant::now() + DEADLINE; + let socket = loop { + match listener.accept() { + Ok((socket, _)) => break socket, + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + if Instant::now() >= deadline { + bail!("TLS fixture did not receive a connection"); + } + thread::sleep(Duration::from_millis(1)); + } + Err(error) => return Err(error.into()), + } + }; + socket.set_read_timeout(Some(DEADLINE))?; + socket.set_write_timeout(Some(DEADLINE))?; + let mut stream = StreamOwned::new(ServerConnection::new(config)?, socket); + match protocol { + Protocol::Http1 => { + let mut head = Vec::new(); + while !head.ends_with(b"\r\n\r\n") { + let mut byte = [0]; + match stream.read(&mut byte) { + Ok(0) => bail!("TLS fixture received EOF before request headers"), + Ok(_) => head.push(byte[0]), + Err(_) if !trust_server => return Ok(()), + Err(error) => return Err(error.into()), + } + if head.len() > 64 * 1024 { + bail!("TLS fixture request headers are too large"); + } + } + } + Protocol::Http2 => { + let mut preface = [0; 24]; + stream.read_exact(&mut preface)?; + assert_eq!(&preface, b"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"); + // Empty server SETTINGS frame, then consume the request's + // connection frames and first HEADERS frame on stream 1. + stream.write_all(&[0, 0, 0, 4, 0, 0, 0, 0, 0])?; + stream.flush()?; + loop { + let mut frame = [0; 9]; + stream.read_exact(&mut frame)?; + let length = u32::from_be_bytes([0, frame[0], frame[1], frame[2]]); + assert!(length <= 64 * 1024); + stream.read_exact(&mut vec![0; length as usize])?; + if frame[3] == 1 { + assert_eq!(&frame[5..], &[0, 0, 0, 1]); + break; + } + } + } + } + stream.write_all(response)?; + stream.flush()?; + match ending { + Ending::CloseNotify => { + stream.conn.send_close_notify(); + stream.flush()?; + } + Ending::TcpEof => {} + Ending::InvalidRecord => { + // A complete TLS record with an invalid authentication tag is + // a protocol error, distinct from transport EOF after plaintext. + stream.sock.write_all(&[23, 3, 3, 0, 17])?; + stream.sock.write_all(&[0x55; 17])?; + stream.sock.flush()?; + } + } + // Send TCP EOF and drain any in-flight HTTP/2 SETTINGS ACK. Closing + // a socket with unread data could send RST instead of the intended FIN. + // A client can finish a framed response and close before this shutdown. + if let Err(error) = stream.sock.shutdown(Shutdown::Write) + && error.kind() != std::io::ErrorKind::NotConnected + { + return Err(error.into()); + } + let mut pending = [0; 4096]; + while matches!(stream.sock.read(&mut pending), Ok(n) if n > 0) {} + // StreamOwned does not send close_notify on Drop. + Ok(()) + }); + + let mut easy = Easy2::new(Body::default()); + easy.url(&format!("https://{address}/response"))?; + easy.proxy("")?; + easy.timeout(DEADLINE)?; + easy.http_version(match protocol { + Protocol::Http1 => HttpVersion::V11, + Protocol::Http2 => HttpVersion::V2, + })?; + CurlTlsConfig { + ca_cert: trust_server.then_some(ca), + ..CurlTlsConfig::default() + } + .configure(&mut easy, false)?; + let result = easy.perform(); + let body = std::mem::take(&mut easy.get_mut().0); + drop(easy); + server.join().expect("TLS fixture thread panicked")?; + if let Err(error) = &result { + assert!( + !error.is_operation_timedout(), + "TLS fixture timed out: {error}" + ); + } + Ok((result, body)) +} + +#[test] +fn tls_close_delimited_response_accepts_transport_eof() -> Result<()> { + for ending in [Ending::CloseNotify, Ending::TcpEof] { + let (result, body) = exchange( + b"HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\n\r\nhello", + ending, + true, + )?; + assert!(result.is_ok(), "{ending:?}: {result:?}"); + assert_eq!(body, b"hello"); + } + Ok(()) +} + +#[test] +fn tls_eof_preserves_http_response_framing() -> Result<()> { + for ending in [Ending::CloseNotify, Ending::TcpEof] { + for (response, complete) in [ + ( + &b"HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\nhello"[..], + true, + ), + ( + &b"HTTP/1.1 200 OK\r\nContent-Length: 8\r\n\r\nhello"[..], + false, + ), + ( + &b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n"[..], + true, + ), + ( + &b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n5\r\nhello\r\n"[..], + false, + ), + ] { + let (result, body) = exchange(response, ending, true)?; + assert_eq!(result.is_ok(), complete, "{ending:?}: {result:?}"); + assert_eq!(body, b"hello"); + } + } + Ok(()) +} + +#[test] +fn tls_eof_accepts_complete_empty_and_informational_responses() -> Result<()> { + for ending in [Ending::CloseNotify, Ending::TcpEof] { + for (response, expected) in [ + (&b"HTTP/1.0 200 OK\r\n\r\nhello"[..], &b"hello"[..]), + (&b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"[..], &b""[..]), + (&b"HTTP/1.1 204 No Content\r\n\r\n"[..], &b""[..]), + (&b"HTTP/1.1 103 Early Hints\r\nLink: ; rel=preload\r\n\r\nHTTP/1.1 200 OK\r\n\r\nhello"[..], &b"hello"[..]), + ] { + let (result, body) = exchange(response, ending, true)?; + assert!(result.is_ok(), "{ending:?}: {result:?}"); + assert_eq!(body, expected); + } + } + Ok(()) +} + +#[test] +fn tls_eof_does_not_hide_invalid_tls_records() -> Result<()> { + let (result, _) = exchange( + b"HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\n\r\nhello", + Ending::InvalidRecord, + true, + )?; + assert!(result.is_err(), "invalid TLS record accepted"); + Ok(()) +} + +#[test] +fn tls_eof_rejects_incomplete_response_headers() -> Result<()> { + for ending in [Ending::CloseNotify, Ending::TcpEof] { + for response in [ + &b""[..], + &b"HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\n"[..], + &b"HTTP/1.1 103 Early Hints\r\nLink: ; rel=preload\r\n\r\n"[..], + ] { + let (result, body) = exchange(response, ending, true)?; + assert!( + result.is_err(), + "{ending:?}: incomplete headers accepted: {response:?}" + ); + assert!(body.is_empty()); + } + } + Ok(()) +} + +#[test] +fn tls_eof_preserves_http2_stream_framing() -> Result<()> { + for ending in [Ending::CloseNotify, Ending::TcpEof] { + // HEADERS with END_HEADERS and HPACK :status 200, followed by DATA. + // Only the first response includes the required END_STREAM flag. + for (response, complete) in [ + (&b"\x00\x00\x01\x01\x04\x00\x00\x00\x01\x88\x00\x00\x05\x00\x01\x00\x00\x00\x01hello"[..], true), + (&b"\x00\x00\x01\x01\x04\x00\x00\x00\x01\x88\x00\x00\x05\x00\x00\x00\x00\x00\x01hello"[..], false), + ] { + let (result, body) = exchange_protocol(response, ending, true, Protocol::Http2)?; + assert_eq!(result.is_ok(), complete, "{ending:?}: {result:?}"); + assert_eq!(body, b"hello"); + } + } + Ok(()) +} + +#[test] +fn tls_eof_does_not_hide_certificate_errors() -> Result<()> { + let (result, body) = exchange(b"HTTP/1.1 200 OK\r\n\r\nhello", Ending::TcpEof, false)?; + assert!( + result.is_err_and(|error| error.is_peer_failed_verification()), + "untrusted self-signed server certificate must fail verification" + ); + assert!(body.is_empty()); + Ok(()) +}