Files
OpenFUT/openfut-http/src/lib.rs
T
funman300 05f6147433 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>
2026-08-11 18:04:08 +00:00

181 lines
6.6 KiB
Rust

//! Request-body handling shared by OpenFUT transport hosts.
//!
//! # Why this is its own crate
//!
//! Draining the request body is not a detail. Answering a client while it is
//! still sending leaves unread data in the receive queue, and Linux turns the
//! subsequent close into an **RST rather than a FIN**. That divergence is
//! invisible to any byte-comparison of the response, and it cost the redirector
//! two live gate attempts before it was found.
//!
//! Once a second host needed the same behaviour, the choice was to share the
//! code or copy it. The same reasoning that produced `openfut-tls` applies:
//! behaviour that must be identical across hosts should have one implementation.
//!
//! # Scope — deliberately narrow
//!
//! Only the part that was **byte-identical** in both hosts is here. The two
//! head-reading loops were *not* identical and are deliberately left where they
//! are:
//!
//! | | redirector | roster |
//! |---|---|---|
//! | head cap | 65536 | 16384 |
//! | read chunk | 4096 | 1024 |
//! | on read error | abort the connection | proceed if any bytes arrived |
//!
//! Those differences may well be accidental, but each host is gate-proven with
//! the values it has. Unifying them would be a behaviour change wearing a
//! refactor's clothes, and this project has already paid for that mistake —
//! the fix is evidence, not tidiness. If they should converge, that is a
//! separate change with its own gate.
//!
//! This crate has no dependencies, knows nothing about TLS, and knows nothing
//! about any game.
use std::io::Read;
/// What happened when the request body was drained.
///
/// Reported per connection because "the client sent a body we never read" is
/// exactly the class of divergence a response-bytes comparison cannot see.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum BodyRead {
/// No `Content-Length` header — nothing to drain.
None,
Complete(usize),
Short {
got: usize,
want: usize,
},
}
impl BodyRead {
/// One-line description for a log or a test failure message.
pub fn describe(self) -> String {
match self {
BodyRead::None => "none".to_string(),
BodyRead::Complete(n) => format!("{n}B complete"),
BodyRead::Short { got, want } => format!("{got}B of {want}B SHORT"),
}
}
}
/// Parse `Content-Length` from the head already in `buf` and read the rest of
/// the body, appending it to `buf`.
///
/// `buf` must already contain everything read so far, including the `\r\n\r\n`
/// terminator and any body bytes that arrived with it.
///
/// Mirrors the Python oracle: header lookup is case-insensitive, and a client
/// that stops early ends the read rather than hanging until the socket timeout.
pub 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::*;
#[test]
fn no_content_length_means_nothing_to_drain() {
let mut buf = b"GET / HTTP/1.1\r\nHost: x\r\n\r\n".to_vec();
let mut rest: &[u8] = b"";
assert_eq!(drain_body(&mut rest, &mut buf), BodyRead::None);
}
#[test]
fn an_unterminated_head_is_not_treated_as_a_body() {
// No \r\n\r\n yet: there is no way to know where a body would start.
let mut buf = b"POST / HTTP/1.1\r\nContent-Length: 4\r\n".to_vec();
let mut rest: &[u8] = b"abcd";
assert_eq!(drain_body(&mut rest, &mut buf), BodyRead::None);
}
#[test]
fn a_body_already_fully_buffered_needs_no_further_read() {
let mut buf = b"POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd".to_vec();
// An empty stream proves nothing more was read.
let mut rest: &[u8] = b"";
assert_eq!(drain_body(&mut rest, &mut buf), BodyRead::Complete(4));
}
#[test]
fn a_body_split_across_reads_is_completed() {
let mut buf = b"POST / HTTP/1.1\r\nContent-Length: 8\r\n\r\nab".to_vec();
let mut rest: &[u8] = b"cdefgh";
assert_eq!(drain_body(&mut rest, &mut buf), BodyRead::Complete(8));
assert!(buf.ends_with(b"abcdefgh"));
}
/// The failure mode that matters: a client that stops early must end the
/// read rather than block. Reported as Short so the host can log it.
#[test]
fn a_client_that_stops_early_is_reported_short() {
let mut buf = b"POST / HTTP/1.1\r\nContent-Length: 10\r\n\r\nabc".to_vec();
let mut rest: &[u8] = b"";
assert_eq!(
drain_body(&mut rest, &mut buf),
BodyRead::Short { got: 3, want: 10 }
);
}
#[test]
fn content_length_is_matched_case_insensitively() {
let mut buf = b"POST / HTTP/1.1\r\ncOnTeNt-LeNgTh: 4\r\n\r\n".to_vec();
let mut rest: &[u8] = b"abcd";
assert_eq!(drain_body(&mut rest, &mut buf), BodyRead::Complete(4));
}
#[test]
fn a_malformed_content_length_is_ignored_rather_than_guessed() {
let mut buf = b"POST / HTTP/1.1\r\nContent-Length: banana\r\n\r\n".to_vec();
let mut rest: &[u8] = b"abcd";
assert_eq!(drain_body(&mut rest, &mut buf), BodyRead::None);
}
/// A header whose NAME merely contains "content-length" must not be read as
/// the real one. `starts_with` is what makes this safe; a `contains` would
/// match `X-Original-Content-Length` and drain the wrong number of bytes.
#[test]
fn a_lookalike_header_is_not_mistaken_for_content_length() {
let mut buf = b"POST / HTTP/1.1\r\nX-Original-Content-Length: 99\r\n\r\n".to_vec();
let mut rest: &[u8] = b"";
assert_eq!(drain_body(&mut rest, &mut buf), BodyRead::None);
}
#[test]
fn describe_is_stable_for_logs() {
assert_eq!(BodyRead::None.describe(), "none");
assert_eq!(BodyRead::Complete(665).describe(), "665B complete");
assert_eq!(
BodyRead::Short { got: 3, want: 10 }.describe(),
"3B of 10B SHORT"
);
}
}