diff --git a/src/stdlib/network/redirect_error_tests.rs b/src/stdlib/network/redirect_error_tests.rs index b90d81566..413355a11 100644 --- a/src/stdlib/network/redirect_error_tests.rs +++ b/src/stdlib/network/redirect_error_tests.rs @@ -7,11 +7,11 @@ //! renames or adds a variant is caught here rather than by a silent fallback to //! `other`. -use std::io::Write as _; use std::time::Duration; use anyhow::{Context, Result, bail, ensure}; use rstest::rstest; +use test_support::http::{self, RawHttpResponse}; use super::*; @@ -92,33 +92,29 @@ fn protocol_failures_are_classified_from_a_live_response() -> Result<()> { /// Drive a hop against a server whose status line cannot be parsed. /// -/// The fixture writes well-formed responses, so this stands a bare listener in -/// its place and answers with a status line no HTTP client accepts. +/// The structured fixture renders only responses a real server could send, so +/// the raw-response fixture supplies the bytes instead. Its URL carries the +/// credentials this hop redacts, and its fixture drains the request before +/// writing and then half-closes, so the malformed bytes are received whole +/// rather than the read failing first. /// /// # Errors /// -/// Returns an error when the listener cannot be bound or read, the malformed -/// response cannot be delivered, or the hop unexpectedly succeeds. +/// Returns an error when the fixture cannot be started or joined, the test URL +/// cannot be parsed, or the hop unexpectedly succeeds. fn malformed_status_line_failure() -> Result { - let listener = std::net::TcpListener::bind(("127.0.0.1", 0)) - .context("bind a malformed-response server")?; - let port = listener - .local_addr() - .context("read the malformed-response server address")? - .port(); - let server = std::thread::spawn(move || -> std::io::Result<()> { - let (mut stream, _addr) = listener.accept()?; - stream.write_all(b"HTTP/1.1 banana OK\r\nContent-Length: 0\r\n\r\n") - }); + let (raw, _log, server) = http::spawn_http_server_raw_response(RawHttpResponse::new( + b"HTTP/1.1 banana OK\r\nContent-Length: 0\r\n\r\n".as_slice(), + )) + .context("spawn a malformed-response server")?; - let raw = format!("http://redirect-user:redirect-secret@127.0.0.1:{port}/start"); - let url = Url::parse(&raw).with_context(|| format!("test URL should parse: {raw}"))?; + let url = Url::parse(&format!("{raw}/start")) + .with_context(|| format!("test URL should parse: {raw}"))?; let agent = build_redirect_agent(); let outcome = request_hop(&agent, &url, Duration::from_secs(5)); server .join() - .map_err(|_panic| anyhow::anyhow!("malformed-response server panicked"))? - .context("write the malformed status line")?; + .map_err(|_panic| anyhow::anyhow!("malformed-response server panicked"))?; let Err(err) = outcome else { bail!("a malformed status line must fail the hop"); }; diff --git a/test_support/src/http/config_tests.rs b/test_support/src/http/config_tests.rs index 59d3a9389..ef8fc9230 100644 --- a/test_support/src/http/config_tests.rs +++ b/test_support/src/http/config_tests.rs @@ -6,7 +6,8 @@ use super::{ ENV_HTTP_ACCEPT_TIMEOUT_MS, ENV_HTTP_POLL_INTERVAL_MS, ENV_HTTP_READ_TIMEOUT_MS, - HttpServerConfig, duration_from_env, take_duration_warnings, + HttpServerConfig, + env::{duration_from_env, take_duration_warnings}, }; use mockable::MockEnv; diff --git a/test_support/src/http/env.rs b/test_support/src/http/env.rs new file mode 100644 index 000000000..03199ca2a --- /dev/null +++ b/test_support/src/http/env.rs @@ -0,0 +1,77 @@ +//! Environment overrides for the fixture's timeouts. +//! +//! The fixture's deadlines are tunable so a slow machine can be given longer +//! than a default that suits a fast one. An override arrives as a string, so +//! this module owns the parsing, the fallback when it does not parse, and the +//! redaction that keeps a caller-supplied value out of the log. + +use std::{fmt, time::Duration}; + +use mockable::Env; + +#[cfg(test)] +use std::{cell::RefCell, thread_local}; + +#[cfg(test)] +thread_local! { + /// Warnings recorded for assertions by the fixture's own tests. + /// + /// A `tracing` subscriber would be the production path, but a unit test + /// cannot observe one without a global subscriber, so the test build + /// captures into a thread-local instead. + pub(super) static DURATION_WARNINGS: RefCell> = + const { RefCell::new(Vec::new()) }; +} + +/// Read `var` as whole milliseconds, falling back to `default` when unset or +/// unparsable. +pub(super) fn duration_from_env(env: &impl Env, var: &str, default: Duration) -> Duration { + env.raw(var).map_or(default, |value| { + let trimmed = value.trim(); + match trimmed.parse::() { + Ok(ms) => Duration::from_millis(ms), + Err(err) => { + log_duration_parse_error(var, trimmed.len(), &err); + default + } + } + }) +} + +/// Report an unparsable duration override without echoing its value. +/// +/// The value is redacted: an environment variable's contents are outside this +/// crate's control, and logging them verbatim would put whatever the caller +/// exported into the log. `err` already names the bounded parse failure, and +/// `value_len` distinguishes an empty override from a malformed one, which is +/// all the diagnosis this fixture needs. +fn log_duration_parse_error(var: &str, value_len: usize, err: &dyn fmt::Display) { + #[cfg(test)] + { + record_duration_warning(format!( + "ignoring invalid {var}: {err} (value redacted, {value_len} bytes)" + )); + } + + #[cfg(not(test))] + { + tracing::warn!( + variable = var, + value_len, + error = %err, + "ignoring invalid fixture duration" + ); + } +} + +/// Record a warning for the fixture's own tests to assert on. +#[cfg(test)] +fn record_duration_warning(message: String) { + DURATION_WARNINGS.with(|warnings| warnings.borrow_mut().push(message)); +} + +/// Take the warnings recorded so far on this thread, leaving the buffer empty. +#[cfg(test)] +pub(super) fn take_duration_warnings() -> Vec { + DURATION_WARNINGS.with(|warnings| warnings.borrow_mut().drain(..).collect()) +} diff --git a/test_support/src/http/mod.rs b/test_support/src/http/mod.rs index ab57d6467..e6585c235 100644 --- a/test_support/src/http/mod.rs +++ b/test_support/src/http/mod.rs @@ -6,8 +6,8 @@ use mockable::{DefaultEnv, Env}; use std::{ - fmt, io, - net::{SocketAddr, TcpListener, TcpStream}, + io, + net::{SocketAddr, TcpStream}, sync::{ Arc, atomic::{AtomicBool, AtomicUsize, Ordering}, @@ -17,14 +17,17 @@ use std::{ }; mod accept; +mod env; mod request; mod response; mod server; +mod spawn; use self::accept::{AcceptWait, accept_connection}; +use self::env::duration_from_env; pub use self::request::RequestLog; -pub use self::response::HttpResponse; -use self::server::{FixtureLedger, run_http_server}; +pub use self::response::{HttpResponse, RawHttpResponse}; +use self::spawn::{spawn_fixture_server, spawn_raw_fixture_server}; /// Override for the timeout in milliseconds within which a client must connect. pub(crate) const ENV_HTTP_ACCEPT_TIMEOUT_MS: &str = "NETSUKE_TEST_HTTP_ACCEPT_TIMEOUT_MS"; @@ -33,14 +36,6 @@ pub(crate) const ENV_HTTP_READ_TIMEOUT_MS: &str = "NETSUKE_TEST_HTTP_READ_TIMEOU /// Override for the polling interval in milliseconds used while waiting. pub(crate) const ENV_HTTP_POLL_INTERVAL_MS: &str = "NETSUKE_TEST_HTTP_POLL_INTERVAL_MS"; -#[cfg(test)] -use std::{cell::RefCell, thread_local}; - -#[cfg(test)] -thread_local! { - static DURATION_WARNINGS: RefCell> = const { RefCell::new(Vec::new()) }; -} - /// Configuration for HTTP fixtures, including timeouts used during polling. #[derive(Debug, Clone)] pub struct HttpServerConfig { @@ -279,97 +274,55 @@ pub fn spawn_http_server_expecting_no_requests( Ok((url, log, server)) } -/// Spawn an HTTP server using `config`, emitting responses in sequence. +/// Spawn an HTTP server that emits each raw response in sequence. /// -/// Returns the bound URL, the shared request count, the request log, and the -/// server handle. The public wrappers above reshape this tuple for their -/// callers, so every fixture shares one server implementation. -fn spawn_fixture_server( - responses: impl IntoIterator, - config: HttpServerConfig, -) -> io::Result<(String, Arc, RequestLog, HttpServer)> { - let response_sequence = responses.into_iter().collect::>(); - let listener = TcpListener::bind(("127.0.0.1", 0))?; - listener.set_nonblocking(true)?; - let addr = listener.local_addr()?; - let url = format!("http://{addr}"); - let requests = Arc::new(AtomicUsize::new(0)); - let server_requests = Arc::clone(&requests); - let log = RequestLog::default(); - let server_log = log.clone(); - let shutdown = Arc::new(AtomicBool::new(false)); - let server_shutdown = Arc::clone(&shutdown); - let handle = thread::Builder::new() - .name("netsuke-http-fixture".into()) - .spawn(move || { - run_http_server( - &listener, - &response_sequence, - &config, - &FixtureLedger::new(&server_requests, &server_log, &server_shutdown), - ); - })?; - Ok(( - url, - requests, - log, - HttpServer { - handle: Some(handle), - addr, - shutdown, - }, - )) -} - -/// Read `var` as whole milliseconds, falling back to `default` when unset or -/// unparsable. -fn duration_from_env(env: &impl Env, var: &str, default: Duration) -> Duration { - env.raw(var).map_or(default, |value| { - let trimmed = value.trim(); - match trimmed.parse::() { - Ok(ms) => Duration::from_millis(ms), - Err(err) => { - log_duration_parse_error(var, trimmed.len(), &err); - default - } - } - }) -} - -/// Report an unparsable duration override without echoing its value. +/// # Raw responses /// -/// The value is redacted: an environment variable's contents are outside this -/// crate's control, and logging them verbatim would put whatever the caller -/// exported into the log. `err` already names the bounded parse failure, and -/// `value_len` distinguishes an empty override from a malformed one, which is -/// all the diagnosis this fixture needs. -fn log_duration_parse_error(var: &str, value_len: usize, err: &dyn fmt::Display) { - #[cfg(test)] - { - record_duration_warning(format!( - "ignoring invalid {var}: {err} (value redacted, {value_len} bytes)" - )); - } - - #[cfg(not(test))] - { - tracing::warn!( - variable = var, - value_len, - error = %err, - "ignoring invalid fixture duration" - ); - } -} - -#[cfg(test)] -fn record_duration_warning(message: String) { - DURATION_WARNINGS.with(|warnings| warnings.borrow_mut().push(message)); +/// The bytes in `responses` are written to the client verbatim, so a caller +/// can emit a status line no HTTP client should accept. This exists because +/// the structured fixtures cannot: [`HttpResponse::new`] takes a status code, +/// so a response whose status line is malformed is not expressible at all, +/// and a test about a client's failure to parse one has to own the bytes. +/// +/// The returned URL carries `redirect-user:redirect-secret` credentials, so a +/// test can drive a credentialed fetch — whose URL redaction is asserted +/// elsewhere — against malformed bytes rather than against a live host. +/// +/// Every raw response is sent over the same fixture machinery as a structured +/// one: the same bounded accept, the same bounded request read, the same +/// accounting and shutdown. The request is drained before the bytes go out, +/// and the write half is then shut down rather than the socket closed, so the +/// response is delivered whole instead of racing a platform-dependent +/// transport abort. See `server::serve_raw_response` for the full reasoning. +/// +/// # Errors +/// +/// Returns an [`io::Error`] if the listener cannot be bound, switched to +/// non-blocking mode, queried for its local address, or if the fixture thread +/// fails to spawn. As with the structured fixtures, later I/O failures inside +/// the fixture thread panic it rather than returning. +pub fn spawn_http_server_raw_responses( + responses: impl IntoIterator, +) -> io::Result<(String, RequestLog, HttpServer)> { + let (url, _requests, log, server) = spawn_raw_fixture_server( + responses, + HttpServerConfig::from_env().accepting_until_shutdown(), + )?; + Ok((url, log, server)) } -#[cfg(test)] -fn take_duration_warnings() -> Vec { - DURATION_WARNINGS.with(|warnings| warnings.borrow_mut().drain(..).collect()) +/// Spawn an HTTP server that emits one raw response. +/// +/// The singular form of [`spawn_http_server_raw_responses`], for the common +/// case of a test that drives exactly one hop against malformed bytes. +/// +/// # Errors +/// +/// Propagates failures while starting the fixture server. +pub fn spawn_http_server_raw_response( + response: RawHttpResponse, +) -> io::Result<(String, RequestLog, HttpServer)> { + spawn_http_server_raw_responses([response]) } #[cfg(test)] diff --git a/test_support/src/http/response.rs b/test_support/src/http/response.rs index 11dc4d1db..d4b67f0d6 100644 --- a/test_support/src/http/response.rs +++ b/test_support/src/http/response.rs @@ -1,4 +1,9 @@ //! HTTP response shapes emitted by the local test fixture. +//! +//! Two shapes are emitted. [`HttpResponse`] describes a valid structured +//! response, which is what most fixtures want; [`RawHttpResponse`] carries the +//! bytes themselves, for tests about what a client does when the bytes on the +//! wire are not a response any server should send. use std::{io, io::Write, net::TcpStream}; @@ -32,6 +37,36 @@ impl HttpResponse { } } +/// Describe one response emitted by the local HTTP fixture as raw bytes. +/// +/// [`HttpResponse`] is the fixture's normal shape: it renders a valid +/// response, so every field it accepts is one a real server could send. A test +/// about what a client does when the bytes on the wire are *not* valid — a +/// status line no client can parse, a truncated header block — cannot be +/// expressed that way, because the constructor refuses the very input the test +/// needs. This type carries the bytes instead, and imposes nothing on them. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RawHttpResponse { + /// Bytes written to the client verbatim, in this order. + bytes: Vec, +} + +impl RawHttpResponse { + /// Create a response that writes `bytes` to the client verbatim. + #[must_use] + pub fn new(bytes: impl Into>) -> Self { + Self { + bytes: bytes.into(), + } + } + + /// Return the bytes this response writes to the client. + #[must_use] + pub fn bytes(&self) -> &[u8] { + &self.bytes + } +} + /// Write `response` to `stream` as a complete HTTP/1.1 response. /// /// # Errors @@ -59,6 +94,18 @@ pub(super) fn render_response(response: &HttpResponse) -> String { ) } +/// Write `response`'s bytes to `stream` verbatim. +/// +/// # Errors +/// +/// Returns an error when the bytes cannot be written to the stream. +pub(super) fn write_raw_response( + stream: &mut TcpStream, + response: &RawHttpResponse, +) -> io::Result<()> { + stream.write_all(&response.bytes) +} + /// Return the standard reason phrase for fixture status codes. const fn reason_phrase(status: u16) -> &'static str { match status { diff --git a/test_support/src/http/server.rs b/test_support/src/http/server.rs index 0206dca33..19245883e 100644 --- a/test_support/src/http/server.rs +++ b/test_support/src/http/server.rs @@ -1,15 +1,22 @@ //! Request-serving implementation for the local HTTP fixture. use std::{ - net::{TcpListener, TcpStream}, + net::{Shutdown, SocketAddr, TcpListener, TcpStream}, sync::atomic::{AtomicBool, AtomicUsize, Ordering}, }; use super::{ - HttpResponse, HttpServerConfig, RequestLog, accept_connection, request::read_request_line, - response, + HttpResponse, HttpServerConfig, RawHttpResponse, RequestLog, accept_connection, + request::read_request_line, response, }; +/// Credentials the raw fixture advertises, and the structured one omits. +/// +/// A test of a credentialed fetch needs userinfo in the URL it drives, and the +/// raw shape is the one such a test reaches for, so the fixture supplies it +/// rather than making every caller paste the same userinfo into its own request. +const RAW_FIXTURE_USERINFO: &str = "redirect-user:redirect-secret"; + /// What one fixture run records about the requests it answers, and the state it /// shares with the test that owns it. /// @@ -44,7 +51,7 @@ impl<'run> FixtureLedger<'run> { /// Report whether the fixture should keep serving configured responses. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum FixtureProgress { +pub(super) enum FixtureProgress { /// The client sent a request and the next response may be served. Continue, /// No further connection is accepted, because either the client @@ -52,49 +59,157 @@ enum FixtureProgress { Shutdown, } -/// Serve the configured responses in request order until one is not requested. -pub(super) fn run_http_server( - listener: &TcpListener, - responses: &[HttpResponse], - config: &HttpServerConfig, - ledger: &FixtureLedger<'_>, -) { - for response in responses { - if serve_fixture_response(listener, response, config, ledger) == FixtureProgress::Shutdown { - return; +/// One accepted client connection, and the response it is owed. +/// +/// The stream and the index travel together because the index is only known +/// once the request has been read: a client that connects and sends nothing +/// must not advance the sequence, so the two cannot be carried separately +/// without inviting a response for a request that never arrived. +#[derive(Debug)] +struct FixtureRequest { + /// The accepted client connection. + stream: TcpStream, + /// Zero-based index, within the configured sequence, of the response owed. + request: usize, +} + +/// What a fixture emits, and how it is driven. +/// +/// This is the whole of the difference between the two shapes the fixture +/// serves: a valid structured response, and bytes the caller supplies. Both are +/// driven by the one loop in [`FixtureServe::drive`], so each shape inherits +/// the same bounded accept, the same bounded request read, the same accounting, +/// and the same shutdown behaviour, rather than reimplementing them. +/// +/// Implementors belong only to the local HTTP fixture, and are moved into the +/// thread that drives them; no production call site may depend on the +/// panic-oriented test failure contract. +pub(super) trait FixtureServe { + /// Serve one response after a client sends a request. + #[must_use] + fn serve_one( + &self, + listener: &TcpListener, + config: &HttpServerConfig, + ledger: &FixtureLedger<'_>, + ) -> FixtureProgress; + + /// Form the fixture URL for a bound loopback address. + #[must_use] + fn advertise(&self, addr: SocketAddr) -> String; + + /// Serve responses in request order until one response is not requested. + /// + /// The single drive loop behind every fixture shape the module exposes. + /// It is defined here, once, rather than in each implementor, so a new + /// shape cannot arrive with its own copy of the run's control flow. + fn drive(&self, listener: &TcpListener, config: &HttpServerConfig, ledger: &FixtureLedger<'_>) { + loop { + if self.serve_one(listener, config, ledger) == FixtureProgress::Shutdown { + return; + } + } + } +} + +/// The response shape a fixture emits, and the URL it advertises. +/// +/// The shapes differ in exactly two ways — the bytes written, and whether the +/// advertised URL carries credentials — so both are one enum, and the one +/// [`FixtureServe`] implementation below dispatches between them. +#[derive(Debug)] +pub(super) enum FixtureResponses { + /// Valid structured responses, advertised without credentials. + Structured(Vec), + /// Bytes written to the client verbatim, advertised with credentials. + Raw(Vec), +} + +impl FixtureResponses { + /// Emit `responses` in request order as valid structured HTTP responses. + #[must_use] + pub(super) const fn structured(responses: Vec) -> Self { + Self::Structured(responses) + } + + /// Emit `responses` in request order as raw bytes. + #[must_use] + pub(super) const fn raw(responses: Vec) -> Self { + Self::Raw(responses) + } +} + +impl FixtureServe for FixtureResponses { + fn serve_one( + &self, + listener: &TcpListener, + config: &HttpServerConfig, + ledger: &FixtureLedger<'_>, + ) -> FixtureProgress { + let Some(mut request) = accept_request(listener, config, ledger) else { + return FixtureProgress::Shutdown; + }; + match self { + Self::Structured(responses) => match responses.get(request.request) { + Some(response) => write_fixture_response(&mut request.stream, response), + // The sequence holds fewer responses than the requests made + // against it, so there is nothing left to send. Ending the run + // keeps the caller's log honest about how far the exchange got. + None => return FixtureProgress::Shutdown, + }, + Self::Raw(responses) => match responses.get(request.request) { + Some(response) => { + write_raw_response(&mut request.stream, response); + finish_raw_response(&request.stream); + } + None => return FixtureProgress::Shutdown, + }, + } + FixtureProgress::Continue + } + + fn advertise(&self, addr: SocketAddr) -> String { + match self { + Self::Structured(_) => format!("http://{addr}"), + Self::Raw(_) => format!("http://{RAW_FIXTURE_USERINFO}@{addr}"), } } } -/// Serve one fixture response after a client sends a non-empty request. +/// Accept one client connection and read the request it sent. /// -/// Returns [`FixtureProgress::Shutdown`] when the client disconnects before -/// sending a request, so an abandoned chain cannot leave later responses -/// waiting on a connection that will never arrive, and likewise when the test -/// shuts the fixture down before any client connects. +/// Returns `None` on the same two conditions as the helpers it composes: no +/// client connected before the test shut the fixture down, or the client +/// disconnected before sending a request. The latter is how an abandoned chain +/// is detected, so a later response cannot wait on a connection that will never +/// arrive. /// -/// This helper belongs only to the local HTTP fixture: `run_http_server` -/// composes it once for every configured response, and no production call site -/// may depend on its panic-oriented test failure contract. -#[must_use] -fn serve_fixture_response( +/// The request is read to a non-empty line *before* any response goes out. That +/// ordering matters, and exists to keep a transport abort from being mistaken +/// for a protocol verdict: a fixture that closes a socket without draining the +/// request can have its close turned into a connection abort by the peer's +/// stack — Windows does exactly this — which the client reports as an I/O +/// failure before it has parsed a status line no parser accepts. The test +/// asserting a parse failure would then be asserting whatever the platform's +/// close semantics happened to do. Draining first removes the unread data, so +/// the bytes are received whole and parsed. A well-formed status line sent this +/// way still arrives; only a malformed one fails, which is the failure under +/// test. +fn accept_request( listener: &TcpListener, - response: &HttpResponse, config: &HttpServerConfig, ledger: &FixtureLedger<'_>, -) -> FixtureProgress { - let Some(mut stream) = accept_fixture_connection(listener, config, ledger.shutdown) else { - return FixtureProgress::Shutdown; - }; +) -> Option { + let mut stream = accept_fixture_connection(listener, config, ledger.shutdown)?; configure_fixture_stream(&stream); - let Some(line) = read_request_line(&mut stream, config.read_deadline(), config.poll_interval) - else { - return FixtureProgress::Shutdown; - }; + let line = read_request_line(&mut stream, config.read_deadline(), config.poll_interval)?; + // Exactly one request line is recorded per accepted connection, and the + // fixture serves one connection at a time, so the log's length before this + // line is recorded is the index of the response this request is owed. + let request = ledger.log.len(); ledger.log.record(line); ledger.requests.fetch_add(1, Ordering::Relaxed); - write_fixture_response(&mut stream, response); - FixtureProgress::Continue + Some(FixtureRequest { stream, request }) } /// Accept one client connection using the fixture configuration. @@ -135,3 +250,54 @@ fn write_fixture_response(stream: &mut TcpStream, response: &HttpResponse) { panic!("failed to write fixture response: {err}"); } } + +/// Write one raw response to a fixture client stream. +#[expect( + clippy::panic, + reason = "test HTTP helper should fail fast when response writing fails" +)] +fn write_raw_response(stream: &mut TcpStream, response: &RawHttpResponse) { + if let Err(err) = response::write_raw_response(stream, response) { + panic!("failed to write raw fixture response: {err}"); + } +} + +/// Mark the end of a raw response by shutting down the stream's write half. +/// +/// A shutdown, not a close: the read half stays open, so the client still has a +/// usable socket to parse from, and the bytes already written are delivered +/// rather than abandoned to whatever a close does to a socket still holding +/// unread peer data. +/// +/// A failure is fatal for every cause except the peer having gone, which is not +/// the fixture's doing and not a verdict on the bytes it sent. A client that +/// has already reset the connection — an abandoned redirect hop, a test that +/// stopped reading — leaves nothing to half-close, and the platform reports +/// that as a transport error (`WSAECONNABORTED` or `ENOTCONN` on Windows, a +/// reset or a disconnection on Unix). Panicking there would turn the client's +/// own departure into a fixture failure, which is the same conflation of +/// transport outcome with protocol verdict this fixture exists to remove. Any +/// other failure means the fixture could not frame what it wrote — the one +/// other reason a client could see a short response — so it still panics. +fn finish_raw_response(stream: &TcpStream) { + if let Err(err) = stream.shutdown(Shutdown::Write) { + assert!( + peer_is_gone(&err), + "failed to shut down the raw fixture response: {err}" + ); + } +} + +/// Report whether `err` is the peer having already left the connection. +/// +/// Deliberately narrow. `BrokenPipe` is excluded: a peer that closed its read +/// half is still there to be answered, so a broken write is the fixture failing +/// to deliver, not the client departing, and must stay fatal. +pub(super) fn peer_is_gone(err: &std::io::Error) -> bool { + use std::io::ErrorKind; + + matches!( + err.kind(), + ErrorKind::NotConnected | ErrorKind::ConnectionReset | ErrorKind::ConnectionAborted + ) +} diff --git a/test_support/src/http/spawn.rs b/test_support/src/http/spawn.rs new file mode 100644 index 000000000..2e3708425 --- /dev/null +++ b/test_support/src/http/spawn.rs @@ -0,0 +1,94 @@ +//! Listener setup and thread ownership for the local HTTP fixture. +//! +//! Every fixture the module exposes is built here, and every one of them is +//! built the same way: bind a loopback port, share a request counter, a request +//! log, and a shutdown flag with the caller, then move a +//! [`FixtureResponses`] into a named thread that serves the responses. What the +//! fixture emits is the only thing a caller varies. + +use std::{ + io, + net::TcpListener, + sync::{ + Arc, + atomic::{AtomicBool, AtomicUsize}, + }, + thread, +}; + +use super::{ + HttpResponse, HttpServer, HttpServerConfig, RawHttpResponse, RequestLog, + server::{FixtureLedger, FixtureResponses, FixtureServe}, +}; + +/// Spawn an HTTP server using `config`, emitting responses in sequence. +/// +/// Returns the bound URL, the shared request count, the request log, and the +/// server handle. The public wrappers in the parent module reshape this tuple +/// for their callers, so every fixture shares one server implementation. +pub(super) fn spawn_fixture_server( + responses: impl IntoIterator, + config: HttpServerConfig, +) -> io::Result<(String, Arc, RequestLog, HttpServer)> { + let response_sequence = responses.into_iter().collect::>(); + spawn_fixture_thread( + Box::new(FixtureResponses::structured(response_sequence)), + config, + ) +} + +/// Spawn an HTTP server using `config`, emitting raw responses in sequence. +/// +/// The raw counterpart of [`spawn_fixture_server`], and the only place the two +/// shapes diverge: both hand a [`FixtureResponses`] to [`spawn_fixture_thread`], +/// which owns binding, accounting, and the thread. +pub(super) fn spawn_raw_fixture_server( + responses: impl IntoIterator, + config: HttpServerConfig, +) -> io::Result<(String, Arc, RequestLog, HttpServer)> { + let response_sequence = responses.into_iter().collect::>(); + spawn_fixture_thread(Box::new(FixtureResponses::raw(response_sequence)), config) +} + +/// Serve `strategy` on a fresh listener, returning its URL and shared state. +/// +/// This is the one implementation behind every fixture the module exposes. A +/// [`FixtureResponses`] supplies what to emit and how to advertise it; everything +/// else — binding a loopback port, the request counter and log shared with the +/// caller, the shutdown flag, the named thread, and the handle that joins or +/// signals them — is common, so no fixture shape can drift from another's +/// acceptance, accounting, or shutdown behaviour. +fn spawn_fixture_thread( + strategy: Box, + config: HttpServerConfig, +) -> io::Result<(String, Arc, RequestLog, HttpServer)> { + let listener = TcpListener::bind(("127.0.0.1", 0))?; + listener.set_nonblocking(true)?; + let addr = listener.local_addr()?; + let url = strategy.advertise(addr); + let requests = Arc::new(AtomicUsize::new(0)); + let server_requests = Arc::clone(&requests); + let log = RequestLog::default(); + let server_log = log.clone(); + let shutdown = Arc::new(AtomicBool::new(false)); + let server_shutdown = Arc::clone(&shutdown); + let handle = thread::Builder::new() + .name("netsuke-http-fixture".into()) + .spawn(move || { + strategy.drive( + &listener, + &config, + &FixtureLedger::new(&server_requests, &server_log, &server_shutdown), + ); + })?; + Ok(( + url, + requests, + log, + HttpServer { + handle: Some(handle), + addr, + shutdown, + }, + )) +} diff --git a/test_support/src/http/tests.rs b/test_support/src/http/tests.rs index 75cf57a81..f9ab84cf8 100644 --- a/test_support/src/http/tests.rs +++ b/test_support/src/http/tests.rs @@ -6,7 +6,8 @@ //! module. use super::{ - AcceptWait, HttpResponse, HttpServerConfig, accept_connection, response::render_response, + AcceptWait, HttpResponse, HttpServerConfig, RawHttpResponse, accept_connection, + response::render_response, }; use std::{ @@ -56,10 +57,7 @@ fn response_server_counts_each_client_request() -> anyhow::Result<()> { /// Send one minimal HTTP request to the fixture at `url`. fn send_request(url: &str) -> anyhow::Result<()> { - let address = url - .strip_prefix("http://") - .ok_or_else(|| anyhow::anyhow!("fixture URL must use HTTP: {url}"))?; - let mut stream = TcpStream::connect(address)?; + let mut stream = connect_fixture(url)?; stream.write_all(b"GET / HTTP/1.1\r\nHost: fixture\r\nConnection: close\r\n\r\n")?; let mut response = String::new(); stream.read_to_string(&mut response)?; @@ -70,6 +68,21 @@ fn send_request(url: &str) -> anyhow::Result<()> { Ok(()) } +/// Connect to the loopback address in a fixture `url`. +/// +/// The raw fixture advertises a URL carrying credentials, which a client must +/// not send as part of the authority, so they are stripped here. The +/// structured fixture advertises none, and takes the same path. +fn connect_fixture(url: &str) -> anyhow::Result { + let authority = url + .strip_prefix("http://") + .ok_or_else(|| anyhow::anyhow!("fixture URL must use HTTP: {url}"))?; + let address = authority + .rsplit_once('@') + .map_or(authority, |(_userinfo, host)| host); + Ok(TcpStream::connect(address)?) +} + #[test] fn accept_connection_respects_accept_timeout() -> anyhow::Result<()> { let listener = TcpListener::bind(("127.0.0.1", 0))?; @@ -209,6 +222,93 @@ fn expect_no_requests_fixture_waits_past_the_accept_timeout() -> anyhow::Result< Ok(()) } +/// A raw response reaches the client byte for byte, framed by a half-close. +/// +/// This is the fixture contract the redirect error tests rest on, and it is +/// asserted here rather than only through those tests because the failure it +/// guards against is silent: a fixture that wrote the bytes and then closed +/// the socket outright could still pass a client-side test on Linux, while +/// aborting the connection on Windows. Reading to EOF is what proves the +/// response was framed — the read terminates on the shutdown, not on a reset — +/// and the exact-byte comparison is what proves nothing was rewritten in +/// flight. The raw bytes are deliberately something no real server should +/// send, because that is the only input the fixture exists to serve. +#[test] +fn raw_response_fixture_delivers_the_exact_bytes_it_was_given() -> anyhow::Result<()> { + let malformed = b"HTTP/1.1 banana OK\r\nContent-Length: 0\r\n\r\n"; + let (url, log, server) = + super::spawn_http_server_raw_response(RawHttpResponse::new(malformed))?; + anyhow::ensure!( + url.contains("redirect-user:redirect-secret@"), + "the raw fixture URL should carry credentials: {url}", + ); + + let received = send_request_and_read_to_eof(&url)?; + server + .join() + .map_err(|err| anyhow::anyhow!("fixture server panicked: {err:?}"))?; + + anyhow::ensure!( + received == malformed, + "the fixture should deliver the bytes it was given, got {:?}", + String::from_utf8_lossy(&received), + ); + anyhow::ensure!( + log.lines() == vec!["GET / HTTP/1.1".to_owned()], + "the raw fixture should record the request it answered: {:?}", + log.lines(), + ); + Ok(()) +} + +/// The raw fixture tolerates exactly one shutdown failure: the peer's absence. +/// +/// Pinned as a predicate rather than through a live reset, because the window +/// between the fixture writing its response and shutting down its write half +/// cannot be lost on purpose from the test side without a sleep, and a +/// sleep-based assertion here would reintroduce the very platform-dependent +/// race this fixture exists to remove. The predicate is the whole of the +/// decision — which transport outcomes the fixture may absorb silently — so +/// asserting it directly is asserting the behaviour. +/// +/// The negative cases matter as much as the positive ones: widening this set +/// would let a fixture that genuinely failed to frame a response report success, +/// which is the silent failure the half-close contract guards against. +#[test] +fn only_a_departed_peer_is_tolerated_when_framing_a_raw_response() { + use std::io::{Error, ErrorKind}; + + for kind in [ + ErrorKind::NotConnected, + ErrorKind::ConnectionReset, + ErrorKind::ConnectionAborted, + ] { + assert!( + super::server::peer_is_gone(&Error::from(kind)), + "{kind:?} is the peer having left, and must not fail the fixture", + ); + } + + for kind in [ErrorKind::BrokenPipe, ErrorKind::TimedOut, ErrorKind::Other] { + assert!( + !super::server::peer_is_gone(&Error::from(kind)), + "{kind:?} is not the peer having left, and must still fail the fixture", + ); + } +} + +/// Send one minimal HTTP request and read the response back to EOF. +/// +/// Unlike [`send_request`], this returns the bytes rather than asserting a +/// status line, because the caller is testing responses that have none. +fn send_request_and_read_to_eof(url: &str) -> anyhow::Result> { + let mut stream = connect_fixture(url)?; + stream.write_all(b"GET / HTTP/1.1\r\nHost: fixture\r\nConnection: close\r\n\r\n")?; + let mut received = Vec::new(); + stream.read_to_end(&mut received)?; + Ok(received) +} + /// The no-request fixture still records a request it receives. /// /// Without this, an empty log would also hold for a fixture that never records