Skip to content

Commit 0990c08

Browse files
committed
fix: validate product references offline and enforce poll deadlines
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
1 parent 50076de commit 0990c08

8 files changed

Lines changed: 1074 additions & 113 deletions

File tree

‎crates/registry-bregctl/src/project_dev/journey.rs‎

Lines changed: 241 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,13 @@ fn execute_step(
156156
.poll
157157
.as_ref()
158158
.map(|poll| Instant::now() + Duration::from_millis(poll.timeout_ms));
159+
let mut last_status = 0;
159160

160161
loop {
161162
active()?;
163+
if poll_deadline.is_some_and(|deadline| Instant::now() >= deadline) {
164+
return Ok((last_status, false, BTreeMap::new()));
165+
}
162166
add_attempt(attempts)?;
163167
add_transfer(transferred, request_bytes)?;
164168
let token = issuer.token(input, person, step.product, clock()? + 300)?;
@@ -180,8 +184,12 @@ fn execute_step(
180184
.header(reqwest::header::CONTENT_TYPE, &spec.content_type)
181185
.body(body.clone());
182186
}
183-
let (response, response_bytes) = runtime.block_on(read_response(request))?;
184-
add_transfer(transferred, response_bytes)?;
187+
let Some(response) =
188+
read_response_before_deadline(runtime, request, poll_deadline, transferred)?
189+
else {
190+
return Ok((last_status, false, BTreeMap::new()));
191+
};
192+
last_status = response.status;
185193
let captures = matches_expectation(step, &response, lookup)?
186194
.then(|| capture_updates(step, &response, lookup))
187195
.and_then(Result::ok);
@@ -443,7 +451,31 @@ fn add_attempt(attempts: &mut usize) -> Result<()> {
443451
Ok(())
444452
}
445453

446-
async fn read_response(request: reqwest::RequestBuilder) -> Result<(ResponseData, usize)> {
454+
fn read_response_before_deadline(
455+
runtime: &tokio::runtime::Runtime,
456+
request: reqwest::RequestBuilder,
457+
deadline: Option<Instant>,
458+
transferred: &mut usize,
459+
) -> Result<Option<ResponseData>> {
460+
let response = read_response(request, transferred);
461+
let Some(deadline) = deadline else {
462+
return runtime.block_on(response).map(Some);
463+
};
464+
if Instant::now() >= deadline {
465+
return Ok(None);
466+
}
467+
let timeout = tokio::time::Instant::from_std(deadline);
468+
match runtime.block_on(async { tokio::time::timeout_at(timeout, response).await }) {
469+
Ok(response) if Instant::now() < deadline => response.map(Some),
470+
Ok(_) => Ok(None),
471+
Err(_) => Ok(None),
472+
}
473+
}
474+
475+
async fn read_response(
476+
request: reqwest::RequestBuilder,
477+
transferred: &mut usize,
478+
) -> Result<ResponseData> {
447479
let mut response = request.send().await?;
448480
let status = response.status().as_u16();
449481
let headers = response.headers().clone();
@@ -454,29 +486,226 @@ async fn read_response(request: reqwest::RequestBuilder) -> Result<(ResponseData
454486
"journey response is too large"
455487
);
456488
bytes.extend_from_slice(&chunk);
489+
add_transfer(transferred, chunk.len())?;
457490
}
458-
let length = bytes.len();
459491
let body = serde_json::from_slice(&bytes).unwrap_or(Value::Null);
460-
Ok((
461-
ResponseData {
462-
status,
463-
headers,
464-
body,
465-
},
466-
length,
467-
))
492+
Ok(ResponseData {
493+
status,
494+
headers,
495+
body,
496+
})
468497
}
469498

470499
#[cfg(test)]
471500
mod tests {
472501
use super::*;
473502
use crate::project_dev::plan::ExpectedStatus;
474503
use serde_json::json;
504+
use std::{
505+
io::{Read, Write},
506+
net::{Ipv4Addr, TcpListener, TcpStream},
507+
path::PathBuf,
508+
sync::{
509+
atomic::{AtomicUsize, Ordering},
510+
Arc, Mutex,
511+
},
512+
};
513+
514+
static POLL_TEST_LOCK: Mutex<()> = Mutex::new(());
475515

476516
fn no_capture(_: &str) -> Result<Value> {
477517
anyhow::bail!("unexpected capture")
478518
}
479519

520+
struct SyntheticServer {
521+
url: String,
522+
requests: Arc<AtomicUsize>,
523+
thread: thread::JoinHandle<()>,
524+
}
525+
526+
impl SyntheticServer {
527+
fn finish(self) -> usize {
528+
self.thread.join().unwrap();
529+
self.requests.load(Ordering::SeqCst)
530+
}
531+
}
532+
533+
fn synthetic_server(responses: Vec<(Duration, u16, &'static str)>) -> SyntheticServer {
534+
let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).unwrap();
535+
listener.set_nonblocking(true).unwrap();
536+
let url = format!("http://{}/", listener.local_addr().unwrap());
537+
let requests = Arc::new(AtomicUsize::new(0));
538+
let observed = requests.clone();
539+
let thread = thread::spawn(move || {
540+
let end = Instant::now() + Duration::from_millis(500);
541+
let mut response = 0;
542+
while Instant::now() < end {
543+
match listener.accept() {
544+
Ok((mut stream, _)) => {
545+
observed.fetch_add(1, Ordering::SeqCst);
546+
stream.set_nonblocking(false).unwrap();
547+
read_request(&mut stream);
548+
let &(delay, status, body) = responses
549+
.get(response)
550+
.unwrap_or_else(|| responses.last().unwrap());
551+
response += 1;
552+
let reason = if status == 200 { "OK" } else { "Accepted" };
553+
let headers = format!(
554+
"HTTP/1.1 {status} {reason}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n",
555+
body.len()
556+
);
557+
if write_to_client(&mut stream, headers.as_bytes()) {
558+
thread::sleep(delay);
559+
write_to_client(&mut stream, body.as_bytes());
560+
}
561+
}
562+
Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => {
563+
thread::sleep(Duration::from_millis(1));
564+
}
565+
Err(error) => panic!("synthetic server accept failed: {error}"),
566+
}
567+
}
568+
});
569+
SyntheticServer {
570+
url,
571+
requests,
572+
thread,
573+
}
574+
}
575+
576+
fn read_request(stream: &mut TcpStream) {
577+
stream
578+
.set_read_timeout(Some(Duration::from_secs(1)))
579+
.unwrap();
580+
let mut request = Vec::new();
581+
let mut chunk = [0u8; 2048];
582+
while request.len() < 16 * 1024 && !request.windows(4).any(|part| part == b"\r\n\r\n") {
583+
match stream.read(&mut chunk) {
584+
Ok(0) | Err(_) => break,
585+
Ok(length) => request.extend_from_slice(&chunk[..length]),
586+
}
587+
}
588+
assert!(request.windows(4).any(|part| part == b"\r\n\r\n"));
589+
}
590+
591+
fn write_to_client(stream: &mut TcpStream, bytes: &[u8]) -> bool {
592+
match stream.write_all(bytes) {
593+
Ok(()) => true,
594+
Err(error)
595+
if matches!(
596+
error.kind(),
597+
std::io::ErrorKind::BrokenPipe
598+
| std::io::ErrorKind::ConnectionAborted
599+
| std::io::ErrorKind::ConnectionReset
600+
| std::io::ErrorKind::NotConnected
601+
) =>
602+
{
603+
false
604+
}
605+
Err(error) => panic!("synthetic server write failed: {error}"),
606+
}
607+
}
608+
609+
fn polling_input(maximum_wait: u64, interval: u64) -> RunInput {
610+
let plan = serde_json::from_value(json!({
611+
"issuer": "https://issuer.example.invalid",
612+
"products": [{"product": "breg", "project": ".", "audience": "registry"}],
613+
"people": {"reader": {
614+
"subject": "reader", "client": "reader-client",
615+
"grants": {"breg": {"profile": "reader", "scopes": ["registry:read"]}}
616+
}},
617+
"journey": {"id": "poll", "steps": [{
618+
"id": "wait", "product": "breg", "as": "reader", "method": "GET",
619+
"path": "/v1/result", "expect": {"status": 200, "json": {"/state": "ready"}},
620+
"poll": {
621+
"maximumWaitMilliseconds": maximum_wait,
622+
"pollIntervalMilliseconds": interval
623+
}
624+
}]}
625+
}))
626+
.unwrap();
627+
RunInput {
628+
run_id: "0123456789abcdef0123456789abcdef".to_owned(),
629+
project: PathBuf::from("/tmp/project"),
630+
plan,
631+
ttl_seconds: 300,
632+
schema_people: Vec::new(),
633+
breg_authority: Some(super::super::BregAuthority {
634+
principal: "sub".to_owned(),
635+
purpose: false,
636+
requires_postgis: false,
637+
named_clients: Default::default(),
638+
}),
639+
casework_principals: BTreeMap::new(),
640+
binaries: BTreeMap::new(),
641+
}
642+
}
643+
644+
#[test]
645+
fn poll_refuses_a_matching_response_that_finishes_after_its_deadline() {
646+
let _lock = POLL_TEST_LOCK
647+
.lock()
648+
.unwrap_or_else(|error| error.into_inner());
649+
let server = synthetic_server(vec![
650+
(Duration::ZERO, 202, r#"{"state":"waiting"}"#),
651+
(Duration::from_millis(500), 200, r#"{"state":"ready"}"#),
652+
]);
653+
let root = tempfile::tempdir().unwrap();
654+
let issuer = Issuer::create(root.path()).unwrap();
655+
let input = polling_input(300, 50);
656+
let urls = BTreeMap::from([(Product::Breg, server.url.clone())]);
657+
658+
let report = run(&input, &issuer, &urls, || Ok(())).unwrap();
659+
660+
assert_eq!(report.result, "failed");
661+
assert_eq!(report.steps[0].status, 202);
662+
assert_eq!(server.finish(), 2);
663+
}
664+
665+
#[test]
666+
fn poll_starts_no_retry_after_its_deadline() {
667+
let _lock = POLL_TEST_LOCK
668+
.lock()
669+
.unwrap_or_else(|error| error.into_inner());
670+
let server = synthetic_server(vec![(Duration::ZERO, 202, r#"{"state":"waiting"}"#)]);
671+
let root = tempfile::tempdir().unwrap();
672+
let issuer = Issuer::create(root.path()).unwrap();
673+
let input = polling_input(300, 300);
674+
let urls = BTreeMap::from([(Product::Breg, server.url.clone())]);
675+
676+
let report = run(&input, &issuer, &urls, || Ok(())).unwrap();
677+
678+
assert_eq!(report.result, "failed");
679+
assert_eq!(report.steps[0].status, 202);
680+
assert_eq!(server.finish(), 1);
681+
}
682+
683+
#[test]
684+
fn poll_sends_nothing_with_zero_remaining_budget() {
685+
let _lock = POLL_TEST_LOCK
686+
.lock()
687+
.unwrap_or_else(|error| error.into_inner());
688+
let server = synthetic_server(vec![(Duration::ZERO, 200, r#"{"state":"ready"}"#)]);
689+
let runtime = tokio::runtime::Builder::new_current_thread()
690+
.enable_all()
691+
.build()
692+
.unwrap();
693+
let request = reqwest::Client::new().get(format!("{}v1/result", server.url));
694+
let mut transferred = 0;
695+
696+
let response = read_response_before_deadline(
697+
&runtime,
698+
request,
699+
Some(Instant::now()),
700+
&mut transferred,
701+
)
702+
.unwrap();
703+
704+
assert!(response.is_none());
705+
assert_eq!(transferred, 0);
706+
assert_eq!(server.finish(), 0);
707+
}
708+
480709
#[test]
481710
fn exact_capture_reference_preserves_scalar_type() {
482711
let captures = BTreeMap::from([("revision", json!(7))]);

0 commit comments

Comments
 (0)