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]