From bba34a9cc126dfcaec09d4297765a36e90a9ee55 Mon Sep 17 00:00:00 2001 From: giswqs Date: Thu, 5 Nov 2026 12:15:07 -0500 Subject: [PATCH 1/3] test(tiles): answer the derived-style test's requests by path The test's server answered exactly five connections in order. A test running at once may clear the shared cache; resolving the style then fetches it again in the background, which took one of the five answers, so the GetFeatureInfo request was refused (macOS CI on main). --- crates/tiles/src/cache.rs | 48 +++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/crates/tiles/src/cache.rs b/crates/tiles/src/cache.rs index 60d98b3c7..e44d0777a 100644 --- a/crates/tiles/src/cache.rs +++ b/crates/tiles/src/cache.rs @@ -2367,6 +2367,35 @@ mod tests { (format!("http://{addr}"), received) } + /// Serves every connection to the returned address with what `answer` gives for its request + /// path, sending each request's head to the receiver. + #[cfg(not(target_arch = "wasm32"))] + fn serve_by_path(answer: impl Fn(&str) -> Vec + Send + 'static) -> (String, std::sync::mpsc::Receiver) { + use std::io::{Read, Write}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let (sent, received) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { return }; + let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(5))); + let mut request = Vec::new(); + let mut buf = [0u8; 2048]; + while !request.windows(4).any(|w| w == b"\r\n\r\n") { + match stream.read(&mut buf) { + Ok(0) | Err(_) => break, + Ok(n) => request.extend_from_slice(&buf[..n]), + } + } + let request = String::from_utf8_lossy(&request).into_owned(); + let path = request.split_whitespace().nth(1).unwrap_or("").to_string(); + let _ = sent.send(request); + let _ = stream.write_all(&answer(&path)); + } + }); + (format!("http://{addr}"), received) + } + #[cfg(not(target_arch = "wasm32"))] #[test] fn logins_headers_follow_same_origin_redirects() { @@ -2621,13 +2650,18 @@ mod tests { let tilejson = serde_json::json!({"tiles": ["./tile/{z}/{x}/{y}.pbf"]}); let answer = |body: String| format!("HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len()).into_bytes(); - let (base, asked) = serve_recorded(vec![ - answer(style.to_string()), - answer(tilejson.to_string()), - answer("{}".into()), - answer("{}".into()), - answer("{}".into()), - ]); + // Answered by path, for as many connections as come: a test running at once may clear the + // shared cache, and resolving then fetches the style again in the background, so the + // requests are neither five nor in a fixed order. + let (base, asked) = serve_by_path(move |path| { + if path.starts_with("/style.json") { + answer(style.to_string()) + } else if path.starts_with("/tiles.json") { + answer(tilejson.to_string()) + } else { + answer("{}".into()) + } + }); let binding = logins::activate(&base, "derived-style", Method::Header("X-Service-Key".into()), "derived-style-secret", true).unwrap(); let url = logins::bind(&binding, &format!("{base}/style.json")).unwrap(); From 0733635052d863a61cf5f045252d8b77253b267e Mon Sep 17 00:00:00 2001 From: giswqs Date: Thu, 5 Nov 2026 12:18:12 -0500 Subject: [PATCH 2/3] Address Claude review feedback - Read the whole request, body too, in one read_request helper that both test servers call, so a future serve_by_path POST isn't reset; note that serve_by_path's thread lives until the test binary exits. --- crates/tiles/src/cache.rs | 69 +++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/crates/tiles/src/cache.rs b/crates/tiles/src/cache.rs index e44d0777a..d71886d5f 100644 --- a/crates/tiles/src/cache.rs +++ b/crates/tiles/src/cache.rs @@ -2329,38 +2329,43 @@ mod tests { serve_recorded(answers).0 } + /// Reads a whole request from `stream`, body too: answering and closing while a POST's body + /// is still arriving resets the connection, and the client sees that instead of the answer. + #[cfg(not(target_arch = "wasm32"))] + fn read_request(stream: &mut std::net::TcpStream) -> String { + use std::io::Read; + let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(5))); + let mut request = Vec::new(); + let mut buf = [0u8; 2048]; + while let Ok(n) = stream.read(&mut buf) { + if n == 0 { + break; + } + request.extend_from_slice(&buf[..n]); + let Some(head_end) = request.windows(4).position(|w| w == b"\r\n\r\n") else { continue }; + let head = String::from_utf8_lossy(&request[..head_end]).to_ascii_lowercase(); + let length = head + .lines() + .find_map(|line| line.strip_prefix("content-length:")) + .and_then(|v| v.trim().parse::().ok()) + .unwrap_or(0); + if request.len() >= head_end + 4 + length { + break; + } + } + String::from_utf8_lossy(&request).into_owned() + } + #[cfg(not(target_arch = "wasm32"))] fn serve_recorded(answers: Vec>) -> (String, std::sync::mpsc::Receiver) { - use std::io::{Read, Write}; + use std::io::Write; let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap(); let (sent, received) = std::sync::mpsc::channel(); std::thread::spawn(move || { for answer in answers { let Ok((mut stream, _)) = listener.accept() else { return }; - // The whole request, body too, before answering: answering and closing while a - // POST's body is still arriving resets the connection, and the client sees that - // instead of the answer. - let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(5))); - let mut request = Vec::new(); - let mut buf = [0u8; 2048]; - while let Ok(n) = stream.read(&mut buf) { - if n == 0 { - break; - } - request.extend_from_slice(&buf[..n]); - let Some(head_end) = request.windows(4).position(|w| w == b"\r\n\r\n") else { continue }; - let head = String::from_utf8_lossy(&request[..head_end]).to_ascii_lowercase(); - let length = head - .lines() - .find_map(|line| line.strip_prefix("content-length:")) - .and_then(|v| v.trim().parse::().ok()) - .unwrap_or(0); - if request.len() >= head_end + 4 + length { - break; - } - } - let _ = sent.send(String::from_utf8_lossy(&request).into_owned()); + let _ = sent.send(read_request(&mut stream)); let _ = stream.write_all(&answer); } }); @@ -2368,26 +2373,18 @@ mod tests { } /// Serves every connection to the returned address with what `answer` gives for its request - /// path, sending each request's head to the receiver. + /// path, sending each request to the receiver. It answers until the test binary exits, so its + /// thread and listener live that long. #[cfg(not(target_arch = "wasm32"))] fn serve_by_path(answer: impl Fn(&str) -> Vec + Send + 'static) -> (String, std::sync::mpsc::Receiver) { - use std::io::{Read, Write}; + use std::io::Write; let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap(); let (sent, received) = std::sync::mpsc::channel(); std::thread::spawn(move || { for stream in listener.incoming() { let Ok(mut stream) = stream else { return }; - let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(5))); - let mut request = Vec::new(); - let mut buf = [0u8; 2048]; - while !request.windows(4).any(|w| w == b"\r\n\r\n") { - match stream.read(&mut buf) { - Ok(0) | Err(_) => break, - Ok(n) => request.extend_from_slice(&buf[..n]), - } - } - let request = String::from_utf8_lossy(&request).into_owned(); + let request = read_request(&mut stream); let path = request.split_whitespace().nth(1).unwrap_or("").to_string(); let _ = sent.send(request); let _ = stream.write_all(&answer(&path)); From 87558dac4c7055af39c668eb066267dd74740747 Mon Sep 17 00:00:00 2001 From: giswqs Date: Thu, 5 Nov 2026 12:55:55 -0500 Subject: [PATCH 3/3] Address Claude review feedback - Check every recorded request, background fetches included, rather than the first five, so the test's own GetFeatureInfo request is always among those checked. --- crates/tiles/src/cache.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/tiles/src/cache.rs b/crates/tiles/src/cache.rs index d71886d5f..2d0b4bf1d 100644 --- a/crates/tiles/src/cache.rs +++ b/crates/tiles/src/cache.rs @@ -2709,7 +2709,11 @@ mod tests { ..Default::default() }; native::get(&native::agent(), &info.request(0.0, 0.0, 10.0).unwrap(), &[]).unwrap(); - for request in asked.iter().take(5) { + // Every request sent so far, background fetches included. Each of the test's own requests + // was sent on before its answer came back, so all of them are among these. + let requests: Vec = asked.try_iter().collect(); + assert!(requests.len() >= 5); + for request in &requests { assert!(request.to_ascii_lowercase().contains("x-service-key: derived-style-secret\r\n")); assert!(!request.contains("rustgis-login:")); }