From 0641110e7a154fd20595e7fd290ef572c8a36235 Mon Sep 17 00:00:00 2001 From: clover caruso Date: Thu, 15 Oct 2026 08:10:05 -0700 Subject: [PATCH] fix(live): a burned code's number is not given out again at once Once a burned code's owner left, the relay forgot the room, so the host's replacement code could draw the same number from the thousand on offer. The old code then led to the new one: a guest typing it was told its code was wrong rather than expired, and spent a try of the new code. A burned room now outlives its members by a minute, during which its number is neither claimed nor joined, and anyone typing the old code hears it expired. The relay test checks this by ending a polled owner, which answers only once it has left, so it needs no waiting. Refs #150 Assisted-by: claude-opus-5.5 --- crates/notebook/tests/live_share.rs | 7 ++-- crates/relay/src/server.rs | 38 +++++++++++++------- crates/relay/tests/relay.rs | 56 +++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 16 deletions(-) diff --git a/crates/notebook/tests/live_share.rs b/crates/notebook/tests/live_share.rs index a98ef1f3546f76192417fc0f30f57212ef2b45bc..31cd05c733cf15877fd9f8d05d55c02aa14db205 100644 --- a/crates/notebook/tests/live_share.rs +++ b/crates/notebook/tests/live_share.rs @@ -424,11 +424,10 @@ fn wrong_codes_are_refused_and_counted() { .is_some_and(|now| now != code && share::code(&now).is_some()) }); let fresh = host.code().unwrap(); - assert!(matches!( - join(&code).unwrap_err(), - Refusal::Expired | Refusal::NoOne - )); + assert_eq!(join(&code).unwrap_err(), Refusal::Expired); + let burned = number; let (number, secret) = notebook::live::code::parse(&fresh).unwrap(); + assert_ne!(number, burned); let wrong = notebook::live::code::format(number, &mistaken(&secret)).unwrap(); assert_eq!(join(&wrong).unwrap_err(), Refusal::Wrong); // A third wrong code in a minute locks this network out, even from the right code. diff --git a/crates/relay/src/server.rs b/crates/relay/src/server.rs index 388312dc25add7d0260a74253b7eb6ae6444617a..d6ab1c0ea849a7adf02482bc3b81b408adff8534 100644 --- a/crates/relay/src/server.rs +++ b/crates/relay/src/server.rs @@ -151,7 +151,9 @@ struct Room { joins: Bucket, bytes: Bucket, failures: u32, - burned: bool, + /// When wrong codes burned it, then when it emptied: its number stays out of use for a + /// minute after, so an old code never leads to its owner's next one. + burned: Option, } struct Member { @@ -429,7 +431,7 @@ impl Relay { let free = |number: u32| { rooms .get(&code(number)) - .is_none_or(|room| room.owner.is_none() && !room.burned) + .is_none_or(|room| room.owner.is_none() && room.burned.is_none()) }; let number = back .filter(|number| NAMEPLATES.contains(number) && free(*number)) @@ -447,13 +449,13 @@ impl Relay { (code(number), Some(number)) } Ask::Room(tag) if tag.starts_with("code-") => { - let room = rooms - .get(&tag) - .filter(|room| room.owner.is_some()) - .ok_or(Refusal::NotFound)?; - if room.burned { + let room = rooms.get(&tag).ok_or(Refusal::NotFound)?; + if room.burned.is_some() { return Err(Refusal::Gone); } + if room.owner.is_none() { + return Err(Refusal::NotFound); + } if client.failures.len() + client.pending >= config.failures_per_minute as usize { let wait = client.failures.front().map_or(config.pending, |first| { (*first + MINUTE).saturating_duration_since(now) @@ -474,7 +476,7 @@ impl Relay { joins: Bucket::full(f64::from(config.room_joins_per_minute), now), bytes: Bucket::full(config.room_bytes_per_second as f64, now), failures: 0, - burned: false, + burned: None, }); let per_minute = f64::from(config.room_joins_per_minute); let admitted = room.members.len() < config.max_room_peers; @@ -649,7 +651,8 @@ impl Relay { } } - /// Fails the peers that waited too long to meet a code's owner, and forgets quiet addresses. + /// Fails the peers that waited too long to meet a code's owner, and forgets burned rooms + /// empty a minute and quiet addresses. fn sweep(&self, now: Instant) { let config = &self.config; let mut state = self.state.lock().unwrap(); @@ -676,6 +679,12 @@ impl Relay { depart(&mut state, config, &poll.tag, poll.slot, now); } } + state.rooms.retain(|_, room| { + !room.members.is_empty() + || room + .burned + .is_some_and(|since| now.saturating_duration_since(since) < MINUTE) + }); state.addresses.retain(|_, address| { address.refresh(now); !address.quiet(config, now) @@ -714,8 +723,8 @@ fn depart(state: &mut State, config: &Config, tag: &str, slot: u32, now: Instant } if member.pending.is_some() { room.failures += 1; - if room.failures >= config.burn_after && !room.burned { - room.burned = true; + if room.failures >= config.burn_after && room.burned.is_none() { + room.burned = Some(now); eprintln!("{tag}: burned after {} wrong codes", room.failures); if let Some(owner) = room.owner.and_then(|owner| room.members.get(&owner)) { owner.outbox.push(notice(Notice::Burned)); @@ -734,7 +743,12 @@ fn depart(state: &mut State, config: &Config, tag: &str, slot: u32, now: Instant } } if room.members.is_empty() { - rooms.remove(tag); + match &mut room.burned { + Some(since) => *since = now, + None => { + rooms.remove(tag); + } + } } } diff --git a/crates/relay/tests/relay.rs b/crates/relay/tests/relay.rs index 240d765708d243dcb436ca141c674435da6ccbcb..d967baa6f069dfccc01d788deee990f4b82b24f4 100644 --- a/crates/relay/tests/relay.rs +++ b/crates/relay/tests/relay.rs @@ -40,6 +40,27 @@ impl Relay { response } + /// Asks `method` on `path` with `body` on a connection of its own: the status line and + /// the answer. + fn ask(&self, method: &str, path: &str, body: &[u8]) -> (String, Vec) { + let mut stream = TcpStream::connect(self.address).unwrap(); + write!( + stream, + "{method} {path} HTTP/1.1\r\nHost: relay\r\nContent-Length: {}\r\n\r\n", + body.len() + ) + .unwrap(); + stream.write_all(body).unwrap(); + let head = ws::head(&mut stream).unwrap(); + let length = ws::header(&head, "Content-Length") + .unwrap() + .parse() + .unwrap(); + let mut answer = vec![0; length]; + stream.read_exact(&mut answer).unwrap(); + (head.lines().next().unwrap().to_owned(), answer) + } + fn join(&self, path: &str) -> Result { self.join_from(path, None) } @@ -299,6 +320,41 @@ fn wrong_codes_burn_a_code() { assert_eq!(status(relay.join(&path)), "HTTP/1.1 410 Gone"); } +/// A burned code's number stays out of use once its owner leaves, so the old code can't lead +/// to the owner's next one. +#[test] +fn a_burned_number_is_not_given_out_again() { + let relay = Relay::start(&["--burn-after", "1"]); + // A polled owner, whose leaving is answered once it has left. + let (_, session) = relay.ask("GET", "/v1/claim?poll=1", b""); + let session = String::from_utf8(session).unwrap(); + let session = format!("/v1/poll/{}", session.strip_prefix("session ").unwrap()); + let say = |opcode: u8, text: &str| { + let frame = ws::frame(opcode, text.as_bytes(), Some([1, 2, 3, 4])); + relay.ask("POST", &session, &frame).0 + }; + let (_, heard) = relay.ask("GET", &session, b""); + let Ok(Message::Text(nameplate)) = ws::Reader::new(&heard[..], 1 << 20, false).read() else { + panic!("heard {heard:?}"); + }; + let number: u32 = nameplate + .strip_prefix("nameplate ") + .unwrap() + .parse() + .unwrap(); + let path = format!("/v1/room/code-{number}"); + let mut guess = relay.join(&path).unwrap(); + assert_eq!(guess.text(), "welcome 2 1"); + assert_eq!(say(ws::TEXT, "failed 2"), "HTTP/1.1 200 OK"); + assert!(guess.closed(Duration::from_secs(5))); + assert_eq!(say(ws::CLOSE, ""), "HTTP/1.1 410 Gone"); + let mut next = relay + .join(&format!("/v1/claim?nameplate={number}")) + .unwrap(); + assert_ne!(next.text(), format!("nameplate {number}")); + assert_eq!(status(relay.join(&path)), "HTTP/1.1 410 Gone"); +} + /// An owner coming back asks for its code's number again, and gets it while no one else /// has claimed it. #[test] -- 2.54.0