http: extract the shared body drain; fix a flaky test race it exposed
Queued cleanup, run only AFTER the roster gate closed in both directions,
so the live A/B changed exactly one thing.
The two `drain_body` implementations were character-for-character
identical, so the extraction is a move. What it guards is not cosmetic:
answering while the client is still sending leaves unread data in the
receive queue and Linux turns the close into an RST rather than a FIN --
invisible in any comparison of the response, and worth two live gate
attempts to find. Behaviour that must be identical across hosts gets one
implementation, the same reasoning that produced openfut-tls.
SCOPE IS DELIBERATELY NARROW. Only the byte-identical part moved. The two
head-reading loops are NOT identical and stay where they are:
redirector roster
head cap 65536 16384
read chunk 4096 1024
on error abort proceed if any bytes arrived
Those differences are probably accidental, but each host is gate-proven
with the values it has. Unifying them would be a behaviour change wearing
a refactor's clothes -- exactly the mistake this project has already paid
for. They converge later as their own change with their own gate, or not
at all.
Purity shown, not asserted: every existing test in both hosts still
passes (426 workspace tests), and 7/7 mutations are killed, including
three in the SHARED crate that must break both hosts at once and one per
host that skips the drain call.
Three test cases neither host had now exist, because the extracted code
finally had somewhere to be tested directly: a malformed Content-Length,
an unterminated head, and a lookalike header. That last one matters --
`X-Original-Content-Length: 99` would drain 99 bytes that were never sent
if the match were `contains` rather than `starts_with`, and a mutation
confirms the test catches it.
Also fixes a race this run exposed in openfut-tls's own tests: keypair()
returned early if the certificate file existed, but wrote the certificate
BEFORE the key, so a parallel test could observe a cert whose key had not
landed. It failed one run and passed the next -- the kind of flake that
gets rerun instead of fixed. Now generated once per process via OnceLock,
key written first, and the suite was repeated five times to confirm.
Nothing deployed and nothing restarted: the running redirector and roster
are still the gate-proven binaries.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -48,6 +48,9 @@ use std::sync::Arc;
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
|
||||
use openfut_adapter_fifa17::roster::{self, Method};
|
||||
// Shared with the redirector: see openfut-http for why this is not duplicated.
|
||||
use openfut_http::drain_body;
|
||||
pub use openfut_http::BodyRead;
|
||||
// The acceptor comes from the shared crate, so this host never names OpenSSL.
|
||||
use openfut_tls::SslAcceptor;
|
||||
|
||||
@@ -92,22 +95,6 @@ pub fn banner(cfg: &RosterConfig) -> String {
|
||||
)
|
||||
}
|
||||
|
||||
/// What happened when the request body was drained.
|
||||
///
|
||||
/// Duplicated from the redirector host on purpose, for now. Unifying the two
|
||||
/// would mean editing the redirector, and the roster A/B must change exactly
|
||||
/// one thing. This is scheduled for extraction once the roster gate closes.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum BodyRead {
|
||||
/// No `Content-Length` header — nothing to drain.
|
||||
None,
|
||||
Complete(usize),
|
||||
Short {
|
||||
got: usize,
|
||||
want: usize,
|
||||
},
|
||||
}
|
||||
|
||||
/// What the host did on a connection, beyond the bytes it sent.
|
||||
///
|
||||
/// Recorded because the divergence that cost two gate attempts on the
|
||||
@@ -283,7 +270,7 @@ fn handle(
|
||||
log(&format!(
|
||||
"conn-{id:04} {peer} {line0} req_bytes={} body={} -> {}B",
|
||||
buf.len(),
|
||||
describe(body),
|
||||
body.describe(),
|
||||
response.len()
|
||||
));
|
||||
record(
|
||||
@@ -308,14 +295,6 @@ fn handle(
|
||||
let _ = tls.shutdown();
|
||||
}
|
||||
|
||||
fn describe(b: BodyRead) -> String {
|
||||
match b {
|
||||
BodyRead::None => "none".into(),
|
||||
BodyRead::Complete(n) => format!("{n}B complete"),
|
||||
BodyRead::Short { got, want } => format!("{got}/{want}B SHORT"),
|
||||
}
|
||||
}
|
||||
|
||||
/// `Date:` for right now, in the oracle's format.
|
||||
fn now_http_date() -> String {
|
||||
let secs = SystemTime::now()
|
||||
@@ -346,38 +325,6 @@ fn read_head<S: Read>(stream: &mut S, buf: &mut Vec<u8>) -> bool {
|
||||
}
|
||||
}
|
||||
|
||||
/// Parse `Content-Length` and read the remainder of the body into `buf`.
|
||||
///
|
||||
/// Header lookup is case-insensitive, and a client that stops early ends the
|
||||
/// read rather than hanging.
|
||||
fn drain_body<S: Read>(stream: &mut S, buf: &mut Vec<u8>) -> BodyRead {
|
||||
let Some(sep) = buf.windows(4).position(|w| w == b"\r\n\r\n") else {
|
||||
return BodyRead::None;
|
||||
};
|
||||
let head = String::from_utf8_lossy(&buf[..sep]).to_string();
|
||||
let Some(want) = head
|
||||
.lines()
|
||||
.find(|l| l.to_ascii_lowercase().starts_with("content-length"))
|
||||
.and_then(|l| l.split(':').nth(1))
|
||||
.and_then(|v| v.trim().parse::<usize>().ok())
|
||||
else {
|
||||
return BodyRead::None;
|
||||
};
|
||||
|
||||
let mut got = buf.len() - (sep + 4);
|
||||
let mut chunk = [0u8; 4096];
|
||||
while got < want {
|
||||
match stream.read(&mut chunk) {
|
||||
Ok(0) | Err(_) => return BodyRead::Short { got, want },
|
||||
Ok(n) => {
|
||||
buf.extend_from_slice(&chunk[..n]);
|
||||
got += n;
|
||||
}
|
||||
}
|
||||
}
|
||||
BodyRead::Complete(got)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
Reference in New Issue
Block a user