Adding upstream version 125.0.1.upstream/125.0.1

Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>

19 files changed, 353 insertions, 178 deletions

diff --git a/third_party/rust/neqo-transport/src/connection/dump.rs b/third_party/rust/neqo-transport/src/connection/dump.rs
index 77d51c605c..8a4f34dbb8 100644
--- a/third_party/rust/neqo-transport/src/connection/dump.rs
+++ b/third_party/rust/neqo-transport/src/connection/dump.rs
@@ -27,11 +27,11 @@ pub fn dump_packet(
     pn: PacketNumber,
     payload: &[u8],
 ) {
-    if !log::log_enabled!(log::Level::Debug) {
+    if log::STATIC_MAX_LEVEL == log::LevelFilter::Off || !log::log_enabled!(log::Level::Debug) {
         return;
     }
 
-    let mut s = String::from("");
+    let mut s = String::new();
     let mut d = Decoder::from(payload);
     while d.remaining() > 0 {
         let Ok(f) = Frame::decode(&mut d) else {
diff --git a/third_party/rust/neqo-transport/src/connection/mod.rs b/third_party/rust/neqo-transport/src/connection/mod.rs
index 2de388418a..c81a3727c6 100644
--- a/third_party/rust/neqo-transport/src/connection/mod.rs
+++ b/third_party/rust/neqo-transport/src/connection/mod.rs
@@ -9,7 +9,6 @@
 use std::{
     cell::RefCell,
     cmp::{max, min},
-    convert::TryFrom,
     fmt::{self, Debug},
     mem,
     net::{IpAddr, SocketAddr},
@@ -23,7 +22,7 @@ use neqo_common::{
     qlog::NeqoQlog, qtrace, qwarn, Datagram, Decoder, Encoder, Role,
 };
 use neqo_crypto::{
-    agent::CertificateInfo, random, Agent, AntiReplay, AuthenticationStatus, Cipher, Client, Group,
+    agent::CertificateInfo, Agent, AntiReplay, AuthenticationStatus, Cipher, Client, Group,
     HandshakeState, PrivateKey, PublicKey, ResumptionToken, SecretAgentInfo, SecretAgentPreInfo,
     Server, ZeroRttChecker,
 };
@@ -48,6 +47,7 @@ use crate::{
     recovery::{LossRecovery, RecoveryToken, SendProfile},
     recv_stream::RecvStreamStats,
     rtt::GRANULARITY,
+    send_stream::SendStream,
     stats::{Stats, StatsCell},
     stream_id::StreamType,
     streams::{SendOrder, Streams},
@@ -59,6 +59,7 @@ use crate::{
     version::{Version, WireVersion},
     AppError, ConnectionError, Error, Res, StreamId,
 };
+
 mod dump;
 mod idle;
 pub mod params;
@@ -66,6 +67,7 @@ mod saved;
 mod state;
 #[cfg(test)]
 pub mod test_internal;
+
 use dump::dump_packet;
 use idle::IdleTimeout;
 pub use params::ConnectionParameters;
@@ -78,9 +80,6 @@ pub use state::{ClosingFrame, State};
 
 pub use crate::send_stream::{RetransmissionPriority, SendStreamStats, TransmissionPriority};
 
-#[derive(Debug, Default)]
-struct Packet(Vec<u8>);
-
 /// The number of Initial packets that the client will send in response
 /// to receiving an undecryptable packet during the early part of the
 /// handshake.  This is a hack, but a useful one.
@@ -96,7 +95,7 @@ pub enum ZeroRttState {
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-/// Type returned from process() and `process_output()`. Users are required to
+/// Type returned from `process()` and `process_output()`. Users are required to
 /// call these repeatedly until `Callback` or `None` is returned.
 pub enum Output {
     /// Connection requires no action.
@@ -119,6 +118,7 @@ impl Output {
     }
 
     /// Get a reference to the Datagram, if any.
+    #[must_use]
     pub fn as_dgram_ref(&self) -> Option<&Datagram> {
         match self {
             Self::Datagram(dg) => Some(dg),
@@ -136,7 +136,7 @@ impl Output {
     }
 }
 
-/// Used by inner functions like Connection::output.
+/// Used by inner functions like `Connection::output`.
 enum SendOption {
     /// Yes, please send this datagram.
     Yes(Datagram),
@@ -257,7 +257,7 @@ pub struct Connection {
     /// Some packets were received, but not tracked.
     received_untracked: bool,
 
-    /// This is responsible for the QuicDatagrams' handling:
+    /// This is responsible for the `QuicDatagrams`' handling:
     /// <https://datatracker.ietf.org/doc/html/draft-ietf-quic-datagram>
     quic_datagrams: QuicDatagrams,
 
@@ -271,8 +271,8 @@ pub struct Connection {
     new_token: NewTokenState,
     stats: StatsCell,
     qlog: NeqoQlog,
-    /// A session ticket was received without NEW_TOKEN,
-    /// this is when that turns into an event without NEW_TOKEN.
+    /// A session ticket was received without `NEW_TOKEN`,
+    /// this is when that turns into an event without `NEW_TOKEN`.
     release_resumption_token_timer: Option<Instant>,
     conn_params: ConnectionParameters,
     hrtime: hrtime::Handle,
@@ -302,6 +302,8 @@ impl Connection {
     const LOOSE_TIMER_RESOLUTION: Duration = Duration::from_millis(50);
 
     /// Create a new QUIC connection with Client role.
+    /// # Errors
+    /// When NSS fails and an agent cannot be created.
     pub fn new_client(
         server_name: impl Into<String>,
         protocols: &[impl AsRef<str>],
@@ -338,6 +340,8 @@ impl Connection {
     }
 
     /// Create a new QUIC connection with Server role.
+    /// # Errors
+    /// When NSS fails and an agent cannot be created.
     pub fn new_server(
         certs: &[impl AsRef<str>],
         protocols: &[impl AsRef<str>],
@@ -427,6 +431,8 @@ impl Connection {
         Ok(c)
     }
 
+    /// # Errors
+    /// When the operation fails.
     pub fn server_enable_0rtt(
         &mut self,
         anti_replay: &AntiReplay,
@@ -436,6 +442,8 @@ impl Connection {
             .server_enable_0rtt(self.tps.clone(), anti_replay, zero_rtt_checker)
     }
 
+    /// # Errors
+    /// When the operation fails.
     pub fn server_enable_ech(
         &mut self,
         config: u8,
@@ -447,10 +455,13 @@ impl Connection {
     }
 
     /// Get the active ECH configuration, which is empty if ECH is disabled.
+    #[must_use]
     pub fn ech_config(&self) -> &[u8] {
         self.crypto.ech_config()
     }
 
+    /// # Errors
+    /// When the operation fails.    
     pub fn client_enable_ech(&mut self, ech_config_list: impl AsRef<[u8]>) -> Res<()> {
         self.crypto.client_enable_ech(ech_config_list)
     }
@@ -468,8 +479,9 @@ impl Connection {
     }
 
     /// Get the original destination connection id for this connection. This
-    /// will always be present for Role::Client but not if Role::Server is in
-    /// State::Init.
+    /// will always be present for `Role::Client` but not if `Role::Server` is in
+    /// `State::Init`.
+    #[must_use]
     pub fn odcid(&self) -> Option<&ConnectionId> {
         self.original_destination_cid.as_ref()
     }
@@ -478,8 +490,9 @@ impl Connection {
     /// This only sets transport parameters without dealing with other aspects of
     /// setting the value.
     ///
+    /// # Errors
+    /// When the transport parameter is invalid.
     /// # Panics
-    ///
     /// This panics if the transport parameter is known to this crate.
     pub fn set_local_tparam(&self, tp: TransportParameterId, value: TransportParameter) -> Res<()> {
         #[cfg(not(test))]
@@ -502,9 +515,9 @@ impl Connection {
     /// Retry.
     pub(crate) fn set_retry_cids(
         &mut self,
-        odcid: ConnectionId,
+        odcid: &ConnectionId,
         remote_cid: ConnectionId,
-        retry_cid: ConnectionId,
+        retry_cid: &ConnectionId,
     ) {
         debug_assert_eq!(self.role, Role::Server);
         qtrace!(
@@ -533,12 +546,16 @@ impl Connection {
 
     /// Set ALPN preferences. Strings that appear earlier in the list are given
     /// higher preference.
+    /// # Errors
+    /// When the operation fails, which is usually due to bad inputs or bad connection state.
     pub fn set_alpn(&mut self, protocols: &[impl AsRef<str>]) -> Res<()> {
         self.crypto.tls.set_alpn(protocols)?;
         Ok(())
     }
 
     /// Enable a set of ciphers.
+    /// # Errors
+    /// When the operation fails, which is usually due to bad inputs or bad connection state.
     pub fn set_ciphers(&mut self, ciphers: &[Cipher]) -> Res<()> {
         if self.state != State::Init {
             qerror!([self], "Cannot enable ciphers in state {:?}", self.state);
@@ -549,6 +566,8 @@ impl Connection {
     }
 
     /// Enable a set of key exchange groups.
+    /// # Errors
+    /// When the operation fails, which is usually due to bad inputs or bad connection state.
     pub fn set_groups(&mut self, groups: &[Group]) -> Res<()> {
         if self.state != State::Init {
             qerror!([self], "Cannot enable groups in state {:?}", self.state);
@@ -559,6 +578,8 @@ impl Connection {
     }
 
     /// Set the number of additional key shares to send in the client hello.
+    /// # Errors
+    /// When the operation fails, which is usually due to bad inputs or bad connection state.
     pub fn send_additional_key_shares(&mut self, count: usize) -> Res<()> {
         if self.state != State::Init {
             qerror!([self], "Cannot enable groups in state {:?}", self.state);
@@ -667,6 +688,8 @@ impl Connection {
     /// This can only be called once and only on the client.
     /// After calling the function, it should be possible to attempt 0-RTT
     /// if the token supports that.
+    /// # Errors
+    /// When the operation fails, which is usually due to bad inputs or bad connection state.
     pub fn enable_resumption(&mut self, now: Instant, token: impl AsRef<[u8]>) -> Res<()> {
         if self.state != State::Init {
             qerror!([self], "set token in state {:?}", self.state);
@@ -683,8 +706,9 @@ impl Connection {
         );
         let mut dec = Decoder::from(token.as_ref());
 
-        let version =
-            Version::try_from(dec.decode_uint(4).ok_or(Error::InvalidResumptionToken)? as u32)?;
+        let version = Version::try_from(u32::try_from(
+            dec.decode_uint(4).ok_or(Error::InvalidResumptionToken)?,
+        )?)?;
         qtrace!([self], "  version {:?}", version);
         if !self.conn_params.get_versions().all().contains(&version) {
             return Err(Error::DisabledVersion);
@@ -732,13 +756,15 @@ impl Connection {
         Ok(())
     }
 
-    pub(crate) fn set_validation(&mut self, validation: Rc<RefCell<AddressValidation>>) {
+    pub(crate) fn set_validation(&mut self, validation: &Rc<RefCell<AddressValidation>>) {
         qtrace!([self], "Enabling NEW_TOKEN");
         assert_eq!(self.role, Role::Server);
-        self.address_validation = AddressValidationInfo::Server(Rc::downgrade(&validation));
+        self.address_validation = AddressValidationInfo::Server(Rc::downgrade(validation));
     }
 
-    /// Send a TLS session ticket AND a NEW_TOKEN frame (if possible).
+    /// Send a TLS session ticket AND a `NEW_TOKEN` frame (if possible).
+    /// # Errors
+    /// When the operation fails, which is usually due to bad inputs or bad connection state.
     pub fn send_ticket(&mut self, now: Instant, extra: &[u8]) -> Res<()> {
         if self.role == Role::Client {
             return Err(Error::WrongRole);
@@ -774,15 +800,19 @@ impl Connection {
         }
     }
 
+    #[must_use]
     pub fn tls_info(&self) -> Option<&SecretAgentInfo> {
         self.crypto.tls.info()
     }
 
+    /// # Errors
+    /// When there is no information to obtain.
     pub fn tls_preinfo(&self) -> Res<SecretAgentPreInfo> {
         Ok(self.crypto.tls.preinfo()?)
     }
 
     /// Get the peer's certificate chain and other info.
+    #[must_use]
     pub fn peer_certificate(&self) -> Option<CertificateInfo> {
         self.crypto.tls.peer_certificate()
     }
@@ -802,26 +832,31 @@ impl Connection {
     }
 
     /// Get the role of the connection.
+    #[must_use]
     pub fn role(&self) -> Role {
         self.role
     }
 
     /// Get the state of the connection.
+    #[must_use]
     pub fn state(&self) -> &State {
         &self.state
     }
 
     /// The QUIC version in use.
+    #[must_use]
     pub fn version(&self) -> Version {
         self.version
     }
 
     /// Get the 0-RTT state of the connection.
+    #[must_use]
     pub fn zero_rtt_state(&self) -> ZeroRttState {
         self.zero_rtt_state
     }
 
     /// Get a snapshot of collected statistics.
+    #[must_use]
     pub fn stats(&self) -> Stats {
         let mut v = self.stats.borrow().clone();
         if let Some(p) = self.paths.primary_fallible() {
@@ -888,7 +923,7 @@ impl Connection {
         res
     }
 
-    /// For use with process_input(). Errors there can be ignored, but this
+    /// For use with `process_input()`. Errors there can be ignored, but this
     /// needs to ensure that the state is updated.
     fn absorb_error<T>(&mut self, now: Instant, res: Res<T>) -> Option<T> {
         self.capture_error(None, now, 0, res).ok()
@@ -1234,6 +1269,7 @@ impl Connection {
 
     /// Perform any processing that we might have to do on packets prior to
     /// attempting to remove protection.
+    #[allow(clippy::too_many_lines)] // Yeah, it's a work in progress.
     fn preprocess_packet(
         &mut self,
         packet: &PublicPacket,
@@ -1346,17 +1382,17 @@ impl Connection {
             }
             State::WaitInitial => PreprocessResult::Continue,
             State::WaitVersion | State::Handshaking | State::Connected | State::Confirmed => {
-                if !self.cid_manager.is_valid(packet.dcid()) {
-                    self.stats
-                        .borrow_mut()
-                        .pkt_dropped(format!("Invalid DCID {:?}", packet.dcid()));
-                    PreprocessResult::Next
-                } else {
+                if self.cid_manager.is_valid(packet.dcid()) {
                     if self.role == Role::Server && packet.packet_type() == PacketType::Handshake {
                         // Server has received a Handshake packet -> discard Initial keys and states
                         self.discard_keys(PacketNumberSpace::Initial, now);
                     }
                     PreprocessResult::Continue
+                } else {
+                    self.stats
+                        .borrow_mut()
+                        .pkt_dropped(format!("Invalid DCID {:?}", packet.dcid()));
+                    PreprocessResult::Next
                 }
             }
             State::Closing { .. } => {
@@ -1376,7 +1412,7 @@ impl Connection {
         Ok(res)
     }
 
-    /// After a Initial, Handshake, ZeroRtt, or Short packet is successfully processed.
+    /// After a Initial, Handshake, `ZeroRtt`, or Short packet is successfully processed.
     fn postprocess_packet(
         &mut self,
         path: &PathRef,
@@ -1576,7 +1612,6 @@ impl Connection {
     /// During connection setup, the first path needs to be setup.
     /// This uses the connection IDs that were provided during the handshake
     /// to setup that path.
-    #[allow(clippy::or_fun_call)] // Remove when MSRV >= 1.59
     fn setup_handshake_path(&mut self, path: &PathRef, now: Instant) {
         self.paths.make_permanent(
             path,
@@ -1616,7 +1651,7 @@ impl Connection {
         }
     }
 
-    /// After an error, a permanent path is needed to send the CONNECTION_CLOSE.
+    /// After an error, a permanent path is needed to send the `CONNECTION_CLOSE`.
     /// This attempts to ensure that this exists.  As the connection is now
     /// temporary, there is no reason to do anything special here.
     fn ensure_error_path(&mut self, path: &PathRef, packet: &PublicPacket, now: Instant) {
@@ -1815,7 +1850,7 @@ impl Connection {
             State::Closing { .. } | State::Draining { .. } | State::Closed(_) => {
                 if let Some(details) = self.state_signaling.close_frame() {
                     let path = Rc::clone(details.path());
-                    let res = self.output_close(details);
+                    let res = self.output_close(&details);
                     self.capture_error(Some(path), now, 0, res)
                 } else {
                     Ok(SendOption::default())
@@ -1892,7 +1927,7 @@ impl Connection {
         }
     }
 
-    fn output_close(&mut self, close: ClosingFrame) -> Res<SendOption> {
+    fn output_close(&mut self, close: &ClosingFrame) -> Res<SendOption> {
         let mut encoder = Encoder::with_capacity(256);
         let grease_quic_bit = self.can_grease_quic_bit();
         let version = self.version();
@@ -1902,6 +1937,14 @@ impl Connection {
             };
 
             let path = close.path().borrow();
+            // In some error cases, we will not be able to make a new, permanent path.
+            // For example, if we run out of connection IDs and the error results from
+            // a packet on a new path, we avoid sending (and the privacy risk) rather
+            // than reuse a connection ID.
+            if path.is_temporary() {
+                assert!(!cfg!(test), "attempting to close with a temporary path");
+                return Err(Error::InternalError);
+            }
             let (_, mut builder) = Self::build_packet_header(
                 &path,
                 cspace,
@@ -1932,7 +1975,7 @@ impl Connection {
             };
             sanitized
                 .as_ref()
-                .unwrap_or(&close)
+                .unwrap_or(close)
                 .write_frame(&mut builder);
             encoder = builder.build(tx)?;
         }
@@ -1946,11 +1989,11 @@ impl Connection {
         &mut self,
         builder: &mut PacketBuilder,
         tokens: &mut Vec<RecoveryToken>,
-    ) -> Res<()> {
+    ) {
         let stats = &mut self.stats.borrow_mut();
         let frame_stats = &mut stats.frame_tx;
         if self.role == Role::Server {
-            if let Some(t) = self.state_signaling.write_done(builder)? {
+            if let Some(t) = self.state_signaling.write_done(builder) {
                 tokens.push(t);
                 frame_stats.handshake_done += 1;
             }
@@ -1959,7 +2002,7 @@ impl Connection {
         self.streams
             .write_frames(TransmissionPriority::Critical, builder, tokens, frame_stats);
         if builder.is_full() {
-            return Ok(());
+            return;
         }
 
         self.streams.write_frames(
@@ -1969,36 +2012,35 @@ impl Connection {
             frame_stats,
         );
         if builder.is_full() {
-            return Ok(());
+            return;
         }
 
         // NEW_CONNECTION_ID, RETIRE_CONNECTION_ID, and ACK_FREQUENCY.
-        self.cid_manager
-            .write_frames(builder, tokens, frame_stats)?;
+        self.cid_manager.write_frames(builder, tokens, frame_stats);
         if builder.is_full() {
-            return Ok(());
+            return;
         }
         self.paths.write_frames(builder, tokens, frame_stats);
         if builder.is_full() {
-            return Ok(());
+            return;
         }
 
         self.streams
             .write_frames(TransmissionPriority::High, builder, tokens, frame_stats);
         if builder.is_full() {
-            return Ok(());
+            return;
         }
 
         self.streams
             .write_frames(TransmissionPriority::Normal, builder, tokens, frame_stats);
         if builder.is_full() {
-            return Ok(());
+            return;
         }
 
         // Datagrams are best-effort and unreliable.  Let streams starve them for now.
         self.quic_datagrams.write_frames(builder, tokens, stats);
         if builder.is_full() {
-            return Ok(());
+            return;
         }
 
         let frame_stats = &mut stats.frame_tx;
@@ -2009,13 +2051,13 @@ impl Connection {
             builder,
             tokens,
             frame_stats,
-        )?;
+        );
         if builder.is_full() {
-            return Ok(());
+            return;
         }
-        self.new_token.write_frames(builder, tokens, frame_stats)?;
+        self.new_token.write_frames(builder, tokens, frame_stats);
         if builder.is_full() {
-            return Ok(());
+            return;
         }
 
         self.streams
@@ -2027,8 +2069,6 @@ impl Connection {
                 w.write_frames(builder);
             }
         }
-
-        Ok(())
     }
 
     // Maybe send a probe.  Return true if the packet was ack-eliciting.
@@ -2089,7 +2129,7 @@ impl Connection {
         profile: &SendProfile,
         builder: &mut PacketBuilder,
         now: Instant,
-    ) -> Res<(Vec<RecoveryToken>, bool, bool)> {
+    ) -> (Vec<RecoveryToken>, bool, bool) {
         let mut tokens = Vec::new();
         let primary = path.borrow().is_primary();
         let mut ack_eliciting = false;
@@ -2125,16 +2165,15 @@ impl Connection {
 
         if profile.ack_only(space) {
             // If we are CC limited we can only send acks!
-            return Ok((tokens, false, false));
+            return (tokens, false, false);
         }
 
         if primary {
             if space == PacketNumberSpace::ApplicationData {
-                self.write_appdata_frames(builder, &mut tokens)?;
+                self.write_appdata_frames(builder, &mut tokens);
             } else {
                 let stats = &mut self.stats.borrow_mut().frame_tx;
-                self.crypto
-                    .write_frame(space, builder, &mut tokens, stats)?;
+                self.crypto.write_frame(space, builder, &mut tokens, stats);
             }
         }
 
@@ -2158,11 +2197,12 @@ impl Connection {
         };
 
         stats.all += tokens.len();
-        Ok((tokens, ack_eliciting, padded))
+        (tokens, ack_eliciting, padded)
     }
 
     /// Build a datagram, possibly from multiple packets (for different PN
     /// spaces) and each containing 1+ frames.
+    #[allow(clippy::too_many_lines)] // Yeah, that's just the way it is.
     fn output_path(&mut self, path: &PathRef, now: Instant) -> Res<SendOption> {
         let mut initial_sent = None;
         let mut needs_padding = false;
@@ -2217,7 +2257,7 @@ impl Connection {
             // Add frames to the packet.
             let payload_start = builder.len();
             let (tokens, ack_eliciting, padded) =
-                self.write_frames(path, *space, &profile, &mut builder, now)?;
+                self.write_frames(path, *space, &profile, &mut builder, now);
             if builder.packet_empty() {
                 // Nothing to include in this packet.
                 encoder = builder.abort();
@@ -2306,6 +2346,8 @@ impl Connection {
         }
     }
 
+    /// # Errors
+    /// When connection state is not valid.
     pub fn initiate_key_update(&mut self) -> Res<()> {
         if self.state == State::Confirmed {
             let la = self
@@ -2319,6 +2361,7 @@ impl Connection {
     }
 
     #[cfg(test)]
+    #[must_use]
     pub fn get_epochs(&self) -> (Option<usize>, Option<usize>) {
         self.crypto.states.get_epochs()
     }
@@ -2377,6 +2420,7 @@ impl Connection {
         );
     }
 
+    #[must_use]
     pub fn is_stream_id_allowed(&self, stream_id: StreamId) -> bool {
         self.streams.is_stream_id_allowed(stream_id)
     }
@@ -2404,7 +2448,7 @@ impl Connection {
             } else {
                 // The other side didn't provide a stateless reset token.
                 // That's OK, they can try guessing this.
-                <[u8; 16]>::try_from(&random(16)[..]).unwrap()
+                ConnectionIdEntry::random_srt()
             };
             self.paths
                 .primary()
@@ -2585,10 +2629,16 @@ impl Connection {
     ) -> Res<()> {
         qtrace!([self], "Handshake space={} data={:0x?}", space, data);
 
+        let was_authentication_pending =
+            *self.crypto.tls.state() == HandshakeState::AuthenticationPending;
         let try_update = data.is_some();
         match self.crypto.handshake(now, space, data)? {
             HandshakeState::Authenticated(_) | HandshakeState::InProgress => (),
-            HandshakeState::AuthenticationPending => self.events.authentication_needed(),
+            HandshakeState::AuthenticationPending => {
+                if !was_authentication_pending {
+                    self.events.authentication_needed();
+                }
+            }
             HandshakeState::EchFallbackAuthenticationPending(public_name) => self
                 .events
                 .ech_fallback_authentication_needed(public_name.clone()),
@@ -2623,6 +2673,7 @@ impl Connection {
         Ok(())
     }
 
+    #[allow(clippy::too_many_lines)] // Yep, but it's a nice big match, which is basically lots of little functions.
     fn input_frame(
         &mut self,
         path: &PathRef,
@@ -2640,7 +2691,7 @@ impl Connection {
         if frame.is_stream() {
             return self
                 .streams
-                .input_frame(frame, &mut self.stats.borrow_mut().frame_rx);
+                .input_frame(&frame, &mut self.stats.borrow_mut().frame_rx);
         }
         match frame {
             Frame::Padding => {
@@ -3005,11 +3056,10 @@ impl Connection {
         Ok(())
     }
 
-    /// Set the SendOrder of a stream.  Re-enqueues to keep the ordering correct
+    /// Set the `SendOrder` of a stream.  Re-enqueues to keep the ordering correct
     ///
     /// # Errors
-    ///
-    /// Returns InvalidStreamId if the stream id doesn't exist
+    /// When the stream does not exist.
     pub fn stream_sendorder(
         &mut self,
         stream_id: StreamId,
@@ -3021,16 +3071,21 @@ impl Connection {
     /// Set the Fairness of a stream
     ///
     /// # Errors
-    ///
-    /// Returns InvalidStreamId if the stream id doesn't exist
+    /// When the stream does not exist.
     pub fn stream_fairness(&mut self, stream_id: StreamId, fairness: bool) -> Res<()> {
         self.streams.set_fairness(stream_id, fairness)
     }
 
+    /// # Errors
+    /// When the stream does not exist.
     pub fn send_stream_stats(&self, stream_id: StreamId) -> Res<SendStreamStats> {
-        self.streams.get_send_stream(stream_id).map(|s| s.stats())
+        self.streams
+            .get_send_stream(stream_id)
+            .map(SendStream::stats)
     }
 
+    /// # Errors
+    /// When the stream does not exist.
     pub fn recv_stream_stats(&mut self, stream_id: StreamId) -> Res<RecvStreamStats> {
         let stream = self.streams.get_recv_stream_mut(stream_id)?;
 
@@ -3050,8 +3105,8 @@ impl Connection {
         self.streams.get_send_stream_mut(stream_id)?.send(data)
     }
 
-    /// Send all data or nothing on a stream. May cause DATA_BLOCKED or
-    /// STREAM_DATA_BLOCKED frames to be sent.
+    /// Send all data or nothing on a stream. May cause `DATA_BLOCKED` or
+    /// `STREAM_DATA_BLOCKED` frames to be sent.
     /// Returns true if data was successfully sent, otherwise false.
     ///
     /// # Errors
@@ -3075,20 +3130,26 @@ impl Connection {
         val.map(|v| v == data.len())
     }
 
-    /// Bytes that stream_send() is guaranteed to accept for sending.
+    /// Bytes that `stream_send()` is guaranteed to accept for sending.
     /// i.e. that will not be blocked by flow credits or send buffer max
     /// capacity.
+    /// # Errors
+    /// When the stream ID is invalid.
     pub fn stream_avail_send_space(&self, stream_id: StreamId) -> Res<usize> {
         Ok(self.streams.get_send_stream(stream_id)?.avail())
     }
 
     /// Close the stream. Enqueued data will be sent.
+    /// # Errors
+    /// When the stream ID is invalid.
     pub fn stream_close_send(&mut self, stream_id: StreamId) -> Res<()> {
         self.streams.get_send_stream_mut(stream_id)?.close();
         Ok(())
     }
 
     /// Abandon transmission of in-flight and future stream data.
+    /// # Errors
+    /// When the stream ID is invalid.
     pub fn stream_reset_send(&mut self, stream_id: StreamId, err: AppError) -> Res<()> {
         self.streams.get_send_stream_mut(stream_id)?.reset(err);
         Ok(())
@@ -3109,6 +3170,8 @@ impl Connection {
     }
 
     /// Application is no longer interested in this stream.
+    /// # Errors
+    /// When the stream ID is invalid.
     pub fn stream_stop_sending(&mut self, stream_id: StreamId, err: AppError) -> Res<()> {
         let stream = self.streams.get_recv_stream_mut(stream_id)?;
 
@@ -3142,6 +3205,7 @@ impl Connection {
         self.streams.keep_alive(stream_id, keep)
     }
 
+    #[must_use]
     pub fn remote_datagram_size(&self) -> u64 {
         self.quic_datagrams.remote_datagram_size()
     }
@@ -3150,9 +3214,10 @@ impl Connection {
     /// The value will change over time depending on the encoded size of the
     /// packet number, ack frames, etc.
     ///
-    /// # Error
-    ///
+    /// # Errors
     /// The function returns `NotAvailable` if datagrams are not enabled.
+    /// # Panics
+    /// Basically never, because that unwrap won't fail.
     pub fn max_datagram_size(&self) -> Res<u64> {
         let max_dgram_size = self.quic_datagrams.remote_datagram_size();
         if max_dgram_size == 0 {
@@ -3193,7 +3258,7 @@ impl Connection {
 
     /// Queue a datagram for sending.
     ///
-    /// # Error
+    /// # Errors
     ///
     /// The function returns `TooMuchData` if the supply buffer is bigger than
     /// the allowed remote datagram size. The funcion does not check if the
@@ -3203,7 +3268,6 @@ impl Connection {
     /// to check the estimated max datagram size and to use smaller datagrams.
     /// `max_datagram_size` is just a current estimate and will change over
     /// time depending on the encoded size of the packet number, ack frames, etc.
-
     pub fn send_datagram(&mut self, buf: &[u8], id: impl Into<DatagramTracking>) -> Res<()> {
         self.quic_datagrams
             .add_datagram(buf, id.into(), &mut self.stats.borrow_mut())
diff --git a/third_party/rust/neqo-transport/src/connection/params.rs b/third_party/rust/neqo-transport/src/connection/params.rs
index 48aba4303b..72d1efa3ee 100644
--- a/third_party/rust/neqo-transport/src/connection/params.rs
+++ b/third_party/rust/neqo-transport/src/connection/params.rs
@@ -4,7 +4,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{cmp::max, convert::TryFrom, time::Duration};
+use std::{cmp::max, time::Duration};
 
 pub use crate::recovery::FAST_PTO_SCALE;
 use crate::{
@@ -41,7 +41,7 @@ pub enum PreferredAddressConfig {
     Address(PreferredAddress),
 }
 
-/// ConnectionParameters use for setting intitial value for QUIC parameters.
+/// `ConnectionParameters` use for setting intitial value for QUIC parameters.
 /// This collects configuration like initial limits, protocol version, and
 /// congestion control algorithm.
 #[derive(Debug, Clone)]
@@ -108,6 +108,7 @@ impl Default for ConnectionParameters {
 }
 
 impl ConnectionParameters {
+    #[must_use]
     pub fn get_versions(&self) -> &VersionConfig {
         &self.versions
     }
@@ -120,29 +121,35 @@ impl ConnectionParameters {
     /// versions that should be enabled.  This list should contain the initial
     /// version and be in order of preference, with more preferred versions
     /// before less preferred.
+    #[must_use]
     pub fn versions(mut self, initial: Version, all: Vec<Version>) -> Self {
         self.versions = VersionConfig::new(initial, all);
         self
     }
 
+    #[must_use]
     pub fn get_cc_algorithm(&self) -> CongestionControlAlgorithm {
         self.cc_algorithm
     }
 
+    #[must_use]
     pub fn cc_algorithm(mut self, v: CongestionControlAlgorithm) -> Self {
         self.cc_algorithm = v;
         self
     }
 
+    #[must_use]
     pub fn get_max_data(&self) -> u64 {
         self.max_data
     }
 
+    #[must_use]
     pub fn max_data(mut self, v: u64) -> Self {
         self.max_data = v;
         self
     }
 
+    #[must_use]
     pub fn get_max_streams(&self, stream_type: StreamType) -> u64 {
         match stream_type {
             StreamType::BiDi => self.max_streams_bidi,
@@ -153,6 +160,7 @@ impl ConnectionParameters {
     /// # Panics
     ///
     /// If v > 2^60 (the maximum allowed by the protocol).
+    #[must_use]
     pub fn max_streams(mut self, stream_type: StreamType, v: u64) -> Self {
         assert!(v <= (1 << 60), "max_streams is too large");
         match stream_type {
@@ -171,6 +179,7 @@ impl ConnectionParameters {
     /// # Panics
     ///
     /// If `StreamType::UniDi` and `false` are passed as that is not a valid combination.
+    #[must_use]
     pub fn get_max_stream_data(&self, stream_type: StreamType, remote: bool) -> u64 {
         match (stream_type, remote) {
             (StreamType::BiDi, false) => self.max_stream_data_bidi_local,
@@ -188,6 +197,7 @@ impl ConnectionParameters {
     ///
     /// If `StreamType::UniDi` and `false` are passed as that is not a valid combination
     /// or if v >= 62 (the maximum allowed by the protocol).
+    #[must_use]
     pub fn max_stream_data(mut self, stream_type: StreamType, remote: bool, v: u64) -> Self {
         assert!(v < (1 << 62), "max stream data is too large");
         match (stream_type, remote) {
@@ -208,26 +218,31 @@ impl ConnectionParameters {
     }
 
     /// Set a preferred address (which only has an effect for a server).
+    #[must_use]
     pub fn preferred_address(mut self, preferred: PreferredAddress) -> Self {
         self.preferred_address = PreferredAddressConfig::Address(preferred);
         self
     }
 
     /// Disable the use of preferred addresses.
+    #[must_use]
     pub fn disable_preferred_address(mut self) -> Self {
         self.preferred_address = PreferredAddressConfig::Disabled;
         self
     }
 
+    #[must_use]
     pub fn get_preferred_address(&self) -> &PreferredAddressConfig {
         &self.preferred_address
     }
 
+    #[must_use]
     pub fn ack_ratio(mut self, ack_ratio: u8) -> Self {
         self.ack_ratio = ack_ratio;
         self
     }
 
+    #[must_use]
     pub fn get_ack_ratio(&self) -> u8 {
         self.ack_ratio
     }
@@ -235,45 +250,54 @@ impl ConnectionParameters {
     /// # Panics
     ///
     /// If `timeout` is 2^62 milliseconds or more.
+    #[must_use]
     pub fn idle_timeout(mut self, timeout: Duration) -> Self {
         assert!(timeout.as_millis() < (1 << 62), "idle timeout is too long");
         self.idle_timeout = timeout;
         self
     }
 
+    #[must_use]
     pub fn get_idle_timeout(&self) -> Duration {
         self.idle_timeout
     }
 
+    #[must_use]
     pub fn get_datagram_size(&self) -> u64 {
         self.datagram_size
     }
 
+    #[must_use]
     pub fn datagram_size(mut self, v: u64) -> Self {
         self.datagram_size = v;
         self
     }
 
+    #[must_use]
     pub fn get_outgoing_datagram_queue(&self) -> usize {
         self.outgoing_datagram_queue
     }
 
+    #[must_use]
     pub fn outgoing_datagram_queue(mut self, v: usize) -> Self {
         // The max queue length must be at least 1.
         self.outgoing_datagram_queue = max(v, 1);
         self
     }
 
+    #[must_use]
     pub fn get_incoming_datagram_queue(&self) -> usize {
         self.incoming_datagram_queue
     }
 
+    #[must_use]
     pub fn incoming_datagram_queue(mut self, v: usize) -> Self {
         // The max queue length must be at least 1.
         self.incoming_datagram_queue = max(v, 1);
         self
     }
 
+    #[must_use]
     pub fn get_fast_pto(&self) -> u8 {
         self.fast_pto
     }
@@ -293,39 +317,50 @@ impl ConnectionParameters {
     /// # Panics
     ///
     /// A value of 0 is invalid and will cause a panic.
+    #[must_use]
     pub fn fast_pto(mut self, scale: u8) -> Self {
         assert_ne!(scale, 0);
         self.fast_pto = scale;
         self
     }
 
+    #[must_use]
     pub fn is_fuzzing(&self) -> bool {
         self.fuzzing
     }
 
+    #[must_use]
     pub fn fuzzing(mut self, enable: bool) -> Self {
         self.fuzzing = enable;
         self
     }
 
+    #[must_use]
     pub fn is_greasing(&self) -> bool {
         self.grease
     }
 
+    #[must_use]
     pub fn grease(mut self, grease: bool) -> Self {
         self.grease = grease;
         self
     }
 
+    #[must_use]
     pub fn pacing_enabled(&self) -> bool {
         self.pacing
     }
 
+    #[must_use]
     pub fn pacing(mut self, pacing: bool) -> Self {
         self.pacing = pacing;
         self
     }
 
+    /// # Errors
+    /// When a connection ID cannot be obtained.
+    /// # Panics
+    /// Only when this code includes a transport parameter that is invalid.
     pub fn create_transport_parameter(
         &self,
         role: Role,
diff --git a/third_party/rust/neqo-transport/src/connection/state.rs b/third_party/rust/neqo-transport/src/connection/state.rs
index 9afb42174f..9789151d3f 100644
--- a/third_party/rust/neqo-transport/src/connection/state.rs
+++ b/third_party/rust/neqo-transport/src/connection/state.rs
@@ -21,7 +21,7 @@ use crate::{
     packet::PacketBuilder,
     path::PathRef,
     recovery::RecoveryToken,
-    ConnectionError, Error, Res,
+    ConnectionError, Error,
 };
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -66,6 +66,7 @@ impl State {
         )
     }
 
+    #[must_use]
     pub fn error(&self) -> Option<&ConnectionError> {
         if let Self::Closing { error, .. } | Self::Draining { error, .. } | Self::Closed(error) =
             self
@@ -184,13 +185,13 @@ impl ClosingFrame {
     }
 }
 
-/// `StateSignaling` manages whether we need to send HANDSHAKE_DONE and CONNECTION_CLOSE.
+/// `StateSignaling` manages whether we need to send `HANDSHAKE_DONE` and `CONNECTION_CLOSE`.
 /// Valid state transitions are:
-/// * Idle -> HandshakeDone: at the server when the handshake completes
-/// * HandshakeDone -> Idle: when a HANDSHAKE_DONE frame is sent
+/// * Idle -> `HandshakeDone`: at the server when the handshake completes
+/// * `HandshakeDone` -> Idle: when a `HANDSHAKE_DONE` frame is sent
 /// * Idle/HandshakeDone -> Closing/Draining: when closing or draining
-/// * Closing/Draining -> CloseSent: after sending CONNECTION_CLOSE
-/// * CloseSent -> Closing: any time a new CONNECTION_CLOSE is needed
+/// * Closing/Draining -> `CloseSent`: after sending `CONNECTION_CLOSE`
+/// * `CloseSent` -> Closing: any time a new `CONNECTION_CLOSE` is needed
 /// * -> Reset: from any state in case of a stateless reset
 #[derive(Debug, Clone)]
 pub enum StateSignaling {
@@ -214,13 +215,13 @@ impl StateSignaling {
         *self = Self::HandshakeDone;
     }
 
-    pub fn write_done(&mut self, builder: &mut PacketBuilder) -> Res<Option<RecoveryToken>> {
+    pub fn write_done(&mut self, builder: &mut PacketBuilder) -> Option<RecoveryToken> {
         if matches!(self, Self::HandshakeDone) && builder.remaining() >= 1 {
             *self = Self::Idle;
             builder.encode_varint(FRAME_TYPE_HANDSHAKE_DONE);
-            Ok(Some(RecoveryToken::HandshakeDone))
+            Some(RecoveryToken::HandshakeDone)
         } else {
-            Ok(None)
+            None
         }
     }
 
diff --git a/third_party/rust/neqo-transport/src/connection/tests/ackrate.rs b/third_party/rust/neqo-transport/src/connection/tests/ackrate.rs
index 1b83d42acd..f0a1d17cd9 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/ackrate.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/ackrate.rs
@@ -6,7 +6,7 @@
 
 use std::{mem, time::Duration};
 
-use test_fixture::{addr_v4, assertions};
+use test_fixture::{assertions, DEFAULT_ADDR_V4};
 
 use super::{
     super::{ConnectionParameters, ACK_RATIO_SCALE},
@@ -164,7 +164,7 @@ fn migrate_ack_delay() {
     let mut now = connect_rtt_idle(&mut client, &mut server, DEFAULT_RTT);
 
     client
-        .migrate(Some(addr_v4()), Some(addr_v4()), true, now)
+        .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), true, now)
         .unwrap();
 
     let client1 = send_something(&mut client, now);
diff --git a/third_party/rust/neqo-transport/src/connection/tests/cc.rs b/third_party/rust/neqo-transport/src/connection/tests/cc.rs
index b3467ea67c..b708bc421d 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/cc.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/cc.rs
@@ -4,7 +4,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{convert::TryFrom, mem, time::Duration};
+use std::{mem, time::Duration};
 
 use neqo_common::{qdebug, qinfo, Datagram};
 
@@ -71,6 +71,7 @@ fn cc_slow_start_to_cong_avoidance_recovery_period() {
         client.stats().frame_rx.largest_acknowledged,
         flight1_largest
     );
+    let cwnd_before_cong = cwnd(&client);
 
     // Client: send more
     let (mut c_tx_dgrams, mut now) = fill_cwnd(&mut client, stream_id, now);
@@ -93,6 +94,7 @@ fn cc_slow_start_to_cong_avoidance_recovery_period() {
         client.stats().frame_rx.largest_acknowledged,
         flight2_largest
     );
+    assert!(cwnd(&client) < cwnd_before_cong);
 }
 
 #[test]
diff --git a/third_party/rust/neqo-transport/src/connection/tests/close.rs b/third_party/rust/neqo-transport/src/connection/tests/close.rs
index f45e77e549..5351dd0d5c 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/close.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/close.rs
@@ -6,7 +6,7 @@
 
 use std::time::Duration;
 
-use test_fixture::{self, datagram, now};
+use test_fixture::{datagram, now};
 
 use super::{
     super::{Connection, Output, State},
diff --git a/third_party/rust/neqo-transport/src/connection/tests/datagram.rs b/third_party/rust/neqo-transport/src/connection/tests/datagram.rs
index 5b7b8dc0b4..ade8c753be 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/datagram.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/datagram.rs
@@ -4,7 +4,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{cell::RefCell, convert::TryFrom, rc::Rc};
+use std::{cell::RefCell, rc::Rc};
 
 use neqo_common::event::Provider;
 use test_fixture::now;
diff --git a/third_party/rust/neqo-transport/src/connection/tests/fuzzing.rs b/third_party/rust/neqo-transport/src/connection/tests/fuzzing.rs
index 5425e1a16e..9924c06fa4 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/fuzzing.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/fuzzing.rs
@@ -4,8 +4,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![cfg_attr(feature = "deny-warnings", deny(warnings))]
-#![warn(clippy::pedantic)]
 #![cfg(feature = "fuzzing")]
 
 use neqo_crypto::FIXED_TAG_FUZZING;
diff --git a/third_party/rust/neqo-transport/src/connection/tests/handshake.rs b/third_party/rust/neqo-transport/src/connection/tests/handshake.rs
index 93385ac1bc..af0352ce90 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/handshake.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/handshake.rs
@@ -6,7 +6,6 @@
 
 use std::{
     cell::RefCell,
-    convert::TryFrom,
     mem,
     net::{IpAddr, Ipv6Addr, SocketAddr},
     rc::Rc,
@@ -18,8 +17,8 @@ use neqo_crypto::{
     constants::TLS_CHACHA20_POLY1305_SHA256, generate_ech_keys, AuthenticationStatus,
 };
 use test_fixture::{
-    self, addr, assertions, assertions::assert_coalesced_0rtt, datagram, fixture_init, now,
-    split_datagram,
+    assertions, assertions::assert_coalesced_0rtt, datagram, fixture_init, now, split_datagram,
+    DEFAULT_ADDR,
 };
 
 use super::{
@@ -122,8 +121,8 @@ fn no_alpn() {
         "example.com",
         &["bad-alpn"],
         Rc::new(RefCell::new(CountingConnectionIdGenerator::default())),
-        addr(),
-        addr(),
+        DEFAULT_ADDR,
+        DEFAULT_ADDR,
         ConnectionParameters::default(),
         now(),
     )
@@ -251,8 +250,8 @@ fn chacha20poly1305() {
         test_fixture::DEFAULT_SERVER_NAME,
         test_fixture::DEFAULT_ALPN,
         Rc::new(RefCell::new(EmptyConnectionIdGenerator::default())),
-        addr(),
-        addr(),
+        DEFAULT_ADDR,
+        DEFAULT_ADDR,
         ConnectionParameters::default(),
         now(),
     )
@@ -347,7 +346,7 @@ fn reorder_05rtt_with_0rtt() {
     let mut server = default_server();
     let validation = AddressValidation::new(now(), ValidateAddress::NoToken).unwrap();
     let validation = Rc::new(RefCell::new(validation));
-    server.set_validation(Rc::clone(&validation));
+    server.set_validation(&validation);
     let mut now = connect_with_rtt(&mut client, &mut server, now(), RTT);
 
     // Include RTT in sending the ticket or the ticket age reported by the
@@ -730,8 +729,8 @@ fn connect_one_version() {
             test_fixture::DEFAULT_SERVER_NAME,
             test_fixture::DEFAULT_ALPN,
             Rc::new(RefCell::new(CountingConnectionIdGenerator::default())),
-            addr(),
-            addr(),
+            DEFAULT_ADDR,
+            DEFAULT_ADDR,
             ConnectionParameters::default().versions(version, vec![version]),
             now(),
         )
@@ -1135,3 +1134,54 @@ fn implicit_rtt_server() {
     // an RTT estimate from having discarded the Initial packet number space.
     assert_eq!(server.stats().rtt, RTT);
 }
+
+#[test]
+fn emit_authentication_needed_once() {
+    let mut client = default_client();
+
+    let mut server = Connection::new_server(
+        test_fixture::LONG_CERT_KEYS,
+        test_fixture::DEFAULT_ALPN,
+        Rc::new(RefCell::new(CountingConnectionIdGenerator::default())),
+        ConnectionParameters::default(),
+    )
+    .expect("create a server");
+
+    let client1 = client.process(None, now());
+    assert!(client1.as_dgram_ref().is_some());
+
+    // The entire server flight doesn't fit in a single packet because the
+    // certificate is large, therefore the server will produce 2 packets.
+    let server1 = server.process(client1.as_dgram_ref(), now());
+    assert!(server1.as_dgram_ref().is_some());
+    let server2 = server.process(None, now());
+    assert!(server2.as_dgram_ref().is_some());
+
+    let authentication_needed_count = |client: &mut Connection| {
+        client
+            .events()
+            .filter(|e| matches!(e, ConnectionEvent::AuthenticationNeeded))
+            .count()
+    };
+
+    // Upon receiving the first packet, the client has the server certificate,
+    // but not yet all required handshake data. It moves to
+    // `HandshakeState::AuthenticationPending` and emits a
+    // `ConnectionEvent::AuthenticationNeeded` event.
+    //
+    // Note that this is a tiny bit fragile in that it depends on having a certificate
+    // that is within a fairly narrow range of sizes.  It has to fit in a single
+    // packet, but be large enough that the CertificateVerify message does not
+    // also fit in the same packet.  Our default test setup achieves this, but
+    // changes to the setup might invalidate this test.
+    let _ = client.process(server1.as_dgram_ref(), now());
+    assert_eq!(1, authentication_needed_count(&mut client));
+    assert!(client.peer_certificate().is_some());
+
+    // The `AuthenticationNeeded` event is still pending a call to
+    // `Connection::authenticated`. On receiving the second packet from the
+    // server, the client must not emit a another
+    // `ConnectionEvent::AuthenticationNeeded`.
+    let _ = client.process(server2.as_dgram_ref(), now());
+    assert_eq!(0, authentication_needed_count(&mut client));
+}
diff --git a/third_party/rust/neqo-transport/src/connection/tests/idle.rs b/third_party/rust/neqo-transport/src/connection/tests/idle.rs
index c33726917a..5d01131541 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/idle.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/idle.rs
@@ -10,7 +10,7 @@ use std::{
 };
 
 use neqo_common::{qtrace, Encoder};
-use test_fixture::{self, now, split_datagram};
+use test_fixture::{now, split_datagram};
 
 use super::{
     super::{Connection, ConnectionParameters, IdleTimeout, Output, State},
@@ -310,28 +310,20 @@ fn idle_caching() {
     server.process_input(&dgram.unwrap(), middle);
     assert_eq!(server.stats().frame_rx.ping, ping_before_s + 1);
     let mut tokens = Vec::new();
-    server
-        .crypto
-        .streams
-        .write_frame(
-            PacketNumberSpace::Initial,
-            &mut builder,
-            &mut tokens,
-            &mut FrameStats::default(),
-        )
-        .unwrap();
+    server.crypto.streams.write_frame(
+        PacketNumberSpace::Initial,
+        &mut builder,
+        &mut tokens,
+        &mut FrameStats::default(),
+    );
     assert_eq!(tokens.len(), 1);
     tokens.clear();
-    server
-        .crypto
-        .streams
-        .write_frame(
-            PacketNumberSpace::Initial,
-            &mut builder,
-            &mut tokens,
-            &mut FrameStats::default(),
-        )
-        .unwrap();
+    server.crypto.streams.write_frame(
+        PacketNumberSpace::Initial,
+        &mut builder,
+        &mut tokens,
+        &mut FrameStats::default(),
+    );
     assert!(tokens.is_empty());
     let dgram = server.process_output(middle).dgram();
 
diff --git a/third_party/rust/neqo-transport/src/connection/tests/keys.rs b/third_party/rust/neqo-transport/src/connection/tests/keys.rs
index c247bba670..847b253284 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/keys.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/keys.rs
@@ -7,7 +7,7 @@
 use std::mem;
 
 use neqo_common::{qdebug, Datagram};
-use test_fixture::{self, now};
+use test_fixture::now;
 
 use super::{
     super::{
diff --git a/third_party/rust/neqo-transport/src/connection/tests/migration.rs b/third_party/rust/neqo-transport/src/connection/tests/migration.rs
index 8307a7dd84..405ae161a4 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/migration.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/migration.rs
@@ -6,6 +6,7 @@
 
 use std::{
     cell::RefCell,
+    mem,
     net::{IpAddr, Ipv6Addr, SocketAddr},
     rc::Rc,
     time::{Duration, Instant},
@@ -13,9 +14,8 @@ use std::{
 
 use neqo_common::{Datagram, Decoder};
 use test_fixture::{
-    self, addr, addr_v4,
     assertions::{assert_v4_path, assert_v6_path},
-    fixture_init, new_neqo_qlog, now,
+    fixture_init, new_neqo_qlog, now, DEFAULT_ADDR, DEFAULT_ADDR_V4,
 };
 
 use super::{
@@ -94,8 +94,8 @@ fn rebinding_port() {
     server.stream_close_send(stream_id).unwrap();
     let dgram = server.process_output(now()).dgram();
     let dgram = dgram.unwrap();
-    assert_eq!(dgram.source(), addr());
-    assert_eq!(dgram.destination(), new_port(addr()));
+    assert_eq!(dgram.source(), DEFAULT_ADDR);
+    assert_eq!(dgram.destination(), new_port(DEFAULT_ADDR));
 }
 
 /// This simulates an attack where a valid packet is forwarded on
@@ -109,7 +109,7 @@ fn path_forwarding_attack() {
     let mut now = now();
 
     let dgram = send_something(&mut client, now);
-    let dgram = change_path(&dgram, addr_v4());
+    let dgram = change_path(&dgram, DEFAULT_ADDR_V4);
     server.process_input(&dgram, now);
 
     // The server now probes the new (primary) path.
@@ -188,7 +188,7 @@ fn migrate_immediate() {
     let now = now();
 
     client
-        .migrate(Some(addr_v4()), Some(addr_v4()), true, now)
+        .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), true, now)
         .unwrap();
 
     let client1 = send_something(&mut client, now);
@@ -229,7 +229,7 @@ fn migrate_rtt() {
     let now = connect_rtt_idle(&mut client, &mut server, RTT);
 
     client
-        .migrate(Some(addr_v4()), Some(addr_v4()), true, now)
+        .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), true, now)
         .unwrap();
     // The RTT might be increased for the new path, so allow a little flexibility.
     let rtt = client.paths.rtt();
@@ -245,7 +245,7 @@ fn migrate_immediate_fail() {
     let mut now = now();
 
     client
-        .migrate(Some(addr_v4()), Some(addr_v4()), true, now)
+        .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), true, now)
         .unwrap();
 
     let probe = client.process_output(now).dgram().unwrap();
@@ -293,7 +293,7 @@ fn migrate_same() {
     let now = now();
 
     client
-        .migrate(Some(addr()), Some(addr()), true, now)
+        .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), true, now)
         .unwrap();
 
     let probe = client.process_output(now).dgram().unwrap();
@@ -320,7 +320,7 @@ fn migrate_same_fail() {
     let mut now = now();
 
     client
-        .migrate(Some(addr()), Some(addr()), true, now)
+        .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), true, now)
         .unwrap();
 
     let probe = client.process_output(now).dgram().unwrap();
@@ -375,7 +375,7 @@ fn migration(mut client: Connection) {
     let now = now();
 
     client
-        .migrate(Some(addr_v4()), Some(addr_v4()), false, now)
+        .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), false, now)
         .unwrap();
 
     let probe = client.process_output(now).dgram().unwrap();
@@ -449,8 +449,8 @@ fn migration_client_empty_cid() {
         test_fixture::DEFAULT_SERVER_NAME,
         test_fixture::DEFAULT_ALPN,
         Rc::new(RefCell::new(EmptyConnectionIdGenerator::default())),
-        addr(),
-        addr(),
+        DEFAULT_ADDR,
+        DEFAULT_ADDR,
         ConnectionParameters::default(),
         now(),
     )
@@ -568,22 +568,22 @@ fn preferred_address(hs_client: SocketAddr, hs_server: SocketAddr, preferred: So
 /// Migration works for a new port number.
 #[test]
 fn preferred_address_new_port() {
-    let a = addr();
+    let a = DEFAULT_ADDR;
     preferred_address(a, a, new_port(a));
 }
 
 /// Migration works for a new address too.
 #[test]
 fn preferred_address_new_address() {
-    let mut preferred = addr();
+    let mut preferred = DEFAULT_ADDR;
     preferred.set_ip(IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2)));
-    preferred_address(addr(), addr(), preferred);
+    preferred_address(DEFAULT_ADDR, DEFAULT_ADDR, preferred);
 }
 
 /// Migration works for IPv4 addresses.
 #[test]
 fn preferred_address_new_port_v4() {
-    let a = addr_v4();
+    let a = DEFAULT_ADDR_V4;
     preferred_address(a, a, new_port(a));
 }
 
@@ -623,7 +623,7 @@ fn preferred_address_ignore_loopback() {
 /// A preferred address in the wrong address family is ignored.
 #[test]
 fn preferred_address_ignore_different_family() {
-    preferred_address_ignored(PreferredAddress::new_any(Some(addr_v4()), None));
+    preferred_address_ignored(PreferredAddress::new_any(Some(DEFAULT_ADDR_V4), None));
 }
 
 /// Disabling preferred addresses at the client means that it ignores a perfectly
@@ -631,7 +631,7 @@ fn preferred_address_ignore_different_family() {
 #[test]
 fn preferred_address_disabled_client() {
     let mut client = new_client(ConnectionParameters::default().disable_preferred_address());
-    let mut preferred = addr();
+    let mut preferred = DEFAULT_ADDR;
     preferred.set_ip(IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2)));
     let spa = PreferredAddress::new_any(None, Some(preferred));
     let mut server = new_server(ConnectionParameters::default().preferred_address(spa));
@@ -643,7 +643,7 @@ fn preferred_address_disabled_client() {
 fn preferred_address_empty_cid() {
     fixture_init();
 
-    let spa = PreferredAddress::new_any(None, Some(new_port(addr())));
+    let spa = PreferredAddress::new_any(None, Some(new_port(DEFAULT_ADDR)));
     let res = Connection::new_server(
         test_fixture::DEFAULT_KEYS,
         test_fixture::DEFAULT_ALPN,
@@ -706,33 +706,33 @@ fn preferred_address_client() {
 fn migration_invalid_state() {
     let mut client = default_client();
     assert!(client
-        .migrate(Some(addr()), Some(addr()), false, now())
+        .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now())
         .is_err());
 
     let mut server = default_server();
     assert!(server
-        .migrate(Some(addr()), Some(addr()), false, now())
+        .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now())
         .is_err());
     connect_force_idle(&mut client, &mut server);
 
     assert!(server
-        .migrate(Some(addr()), Some(addr()), false, now())
+        .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now())
         .is_err());
 
     client.close(now(), 0, "closing");
     assert!(client
-        .migrate(Some(addr()), Some(addr()), false, now())
+        .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now())
         .is_err());
     let close = client.process(None, now()).dgram();
 
     let dgram = server.process(close.as_ref(), now()).dgram();
     assert!(server
-        .migrate(Some(addr()), Some(addr()), false, now())
+        .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now())
         .is_err());
 
     client.process_input(&dgram.unwrap(), now());
     assert!(client
-        .migrate(Some(addr()), Some(addr()), false, now())
+        .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now())
         .is_err());
 }
 
@@ -753,32 +753,32 @@ fn migration_invalid_address() {
     cant_migrate(None, None);
 
     // Providing a zero port number isn't valid.
-    let mut zero_port = addr();
+    let mut zero_port = DEFAULT_ADDR;
     zero_port.set_port(0);
     cant_migrate(None, Some(zero_port));
     cant_migrate(Some(zero_port), None);
 
     // An unspecified remote address is bad.
-    let mut remote_unspecified = addr();
+    let mut remote_unspecified = DEFAULT_ADDR;
     remote_unspecified.set_ip(IpAddr::V6(Ipv6Addr::from(0)));
     cant_migrate(None, Some(remote_unspecified));
 
     // Mixed address families is bad.
-    cant_migrate(Some(addr()), Some(addr_v4()));
-    cant_migrate(Some(addr_v4()), Some(addr()));
+    cant_migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR_V4));
+    cant_migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR));
 
     // Loopback to non-loopback is bad.
-    cant_migrate(Some(addr()), Some(loopback()));
-    cant_migrate(Some(loopback()), Some(addr()));
+    cant_migrate(Some(DEFAULT_ADDR), Some(loopback()));
+    cant_migrate(Some(loopback()), Some(DEFAULT_ADDR));
     assert_eq!(
         client
-            .migrate(Some(addr()), Some(loopback()), true, now())
+            .migrate(Some(DEFAULT_ADDR), Some(loopback()), true, now())
             .unwrap_err(),
         Error::InvalidMigration
     );
     assert_eq!(
         client
-            .migrate(Some(loopback()), Some(addr()), true, now())
+            .migrate(Some(loopback()), Some(DEFAULT_ADDR), true, now())
             .unwrap_err(),
         Error::InvalidMigration
     );
@@ -864,7 +864,7 @@ fn retire_prior_to_migration_failure() {
     let original_cid = ConnectionId::from(get_cid(&send_something(&mut client, now())));
 
     client
-        .migrate(Some(addr_v4()), Some(addr_v4()), false, now())
+        .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), false, now())
         .unwrap();
 
     // The client now probes the new path.
@@ -919,7 +919,7 @@ fn retire_prior_to_migration_success() {
     let original_cid = ConnectionId::from(get_cid(&send_something(&mut client, now())));
 
     client
-        .migrate(Some(addr_v4()), Some(addr_v4()), false, now())
+        .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), false, now())
         .unwrap();
 
     // The client now probes the new path.
@@ -951,3 +951,39 @@ fn retire_prior_to_migration_success() {
     assert_ne!(get_cid(&dgram), original_cid);
     assert_ne!(get_cid(&dgram), probe_cid);
 }
+
+struct GarbageWriter {}
+
+impl crate::connection::test_internal::FrameWriter for GarbageWriter {
+    fn write_frames(&mut self, builder: &mut PacketBuilder) {
+        // Not a valid frame type.
+        builder.encode_varint(u32::MAX);
+    }
+}
+
+/// Test the case that we run out of connection ID and receive an invalid frame
+/// from a new path.
+#[test]
+#[should_panic(expected = "attempting to close with a temporary path")]
+fn error_on_new_path_with_no_connection_id() {
+    let mut client = default_client();
+    let mut server = default_server();
+    connect_force_idle(&mut client, &mut server);
+
+    let cid_gen: Rc<RefCell<dyn ConnectionIdGenerator>> =
+        Rc::new(RefCell::new(CountingConnectionIdGenerator::default()));
+    server.test_frame_writer = Some(Box::new(RetireAll { cid_gen }));
+    let retire_all = send_something(&mut server, now());
+
+    client.process_input(&retire_all, now());
+
+    server.test_frame_writer = Some(Box::new(GarbageWriter {}));
+    let garbage = send_something(&mut server, now());
+
+    let dgram = change_path(&garbage, DEFAULT_ADDR_V4);
+    client.process_input(&dgram, now());
+
+    // See issue #1697. We had a crash when the client had a temporary path and
+    // process_output is called.
+    mem::drop(client.process_output(now()));
+}
diff --git a/third_party/rust/neqo-transport/src/connection/tests/mod.rs b/third_party/rust/neqo-transport/src/connection/tests/mod.rs
index 8a999f4048..b6ce08f8d1 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/mod.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/mod.rs
@@ -4,12 +4,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![deny(clippy::pedantic)]
-
 use std::{
     cell::RefCell,
     cmp::min,
-    convert::TryFrom,
     mem,
     rc::Rc,
     time::{Duration, Instant},
@@ -18,7 +15,7 @@ use std::{
 use enum_map::enum_map;
 use neqo_common::{event::Provider, qdebug, qtrace, Datagram, Decoder, Role};
 use neqo_crypto::{random, AllowZeroRtt, AuthenticationStatus, ResumptionToken};
-use test_fixture::{self, addr, fixture_init, new_neqo_qlog, now};
+use test_fixture::{fixture_init, new_neqo_qlog, now, DEFAULT_ADDR};
 
 use super::{Connection, ConnectionError, ConnectionId, Output, State};
 use crate::{
@@ -79,7 +76,7 @@ impl ConnectionIdDecoder for CountingConnectionIdGenerator {
 
 impl ConnectionIdGenerator for CountingConnectionIdGenerator {
     fn generate_cid(&mut self) -> Option<ConnectionId> {
-        let mut r = random(20);
+        let mut r = random::<20>();
         r[0] = 8;
         r[1] = u8::try_from(self.counter >> 24).unwrap();
         r[2] = u8::try_from((self.counter >> 16) & 0xff).unwrap();
@@ -107,8 +104,8 @@ pub fn new_client(params: ConnectionParameters) -> Connection {
         test_fixture::DEFAULT_SERVER_NAME,
         test_fixture::DEFAULT_ALPN,
         Rc::new(RefCell::new(CountingConnectionIdGenerator::default())),
-        addr(),
-        addr(),
+        DEFAULT_ADDR,
+        DEFAULT_ADDR,
         params,
         now(),
     )
@@ -278,7 +275,7 @@ fn exchange_ticket(
 ) -> ResumptionToken {
     let validation = AddressValidation::new(now, ValidateAddress::NoToken).unwrap();
     let validation = Rc::new(RefCell::new(validation));
-    server.set_validation(Rc::clone(&validation));
+    server.set_validation(&validation);
     server.send_ticket(now, &[]).expect("can send ticket");
     let ticket = server.process_output(now).dgram();
     assert!(ticket.is_some());
diff --git a/third_party/rust/neqo-transport/src/connection/tests/priority.rs b/third_party/rust/neqo-transport/src/connection/tests/priority.rs
index 1f86aa22e5..079ba93b9f 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/priority.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/priority.rs
@@ -7,7 +7,7 @@
 use std::{cell::RefCell, mem, rc::Rc};
 
 use neqo_common::event::Provider;
-use test_fixture::{self, now};
+use test_fixture::now;
 
 use super::{
     super::{Connection, Error, Output},
@@ -370,7 +370,7 @@ fn low() {
     let validation = Rc::new(RefCell::new(
         AddressValidation::new(now, ValidateAddress::Never).unwrap(),
     ));
-    server.set_validation(Rc::clone(&validation));
+    server.set_validation(&validation);
     connect(&mut client, &mut server);
 
     let id = server.stream_create(StreamType::UniDi).unwrap();
diff --git a/third_party/rust/neqo-transport/src/connection/tests/resumption.rs b/third_party/rust/neqo-transport/src/connection/tests/resumption.rs
index a8c45a9f06..7410e76ef8 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/resumption.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/resumption.rs
@@ -6,7 +6,7 @@
 
 use std::{cell::RefCell, mem, rc::Rc, time::Duration};
 
-use test_fixture::{self, assertions, now};
+use test_fixture::{assertions, now};
 
 use super::{
     connect, connect_with_rtt, default_client, default_server, exchange_ticket, get_tokens,
@@ -50,7 +50,7 @@ fn remember_smoothed_rtt() {
     // wants to acknowledge; so the ticket will include an ACK frame too.
     let validation = AddressValidation::new(now, ValidateAddress::NoToken).unwrap();
     let validation = Rc::new(RefCell::new(validation));
-    server.set_validation(Rc::clone(&validation));
+    server.set_validation(&validation);
     server.send_ticket(now, &[]).expect("can send ticket");
     let ticket = server.process_output(now).dgram();
     assert!(ticket.is_some());
@@ -84,7 +84,7 @@ fn address_validation_token_resume() {
     let mut server = default_server();
     let validation = AddressValidation::new(now(), ValidateAddress::Always).unwrap();
     let validation = Rc::new(RefCell::new(validation));
-    server.set_validation(Rc::clone(&validation));
+    server.set_validation(&validation);
     let mut now = connect_with_rtt(&mut client, &mut server, now(), RTT);
 
     let token = exchange_ticket(&mut client, &mut server, now);
@@ -155,7 +155,7 @@ fn two_tickets_with_new_token() {
     let mut server = default_server();
     let validation = AddressValidation::new(now(), ValidateAddress::Always).unwrap();
     let validation = Rc::new(RefCell::new(validation));
-    server.set_validation(Rc::clone(&validation));
+    server.set_validation(&validation);
     connect(&mut client, &mut server);
 
     // Send two tickets with tokens and then bundle those into a packet.
diff --git a/third_party/rust/neqo-transport/src/connection/tests/stream.rs b/third_party/rust/neqo-transport/src/connection/tests/stream.rs
index 586a537b9d..f469866d50 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/stream.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/stream.rs
@@ -4,7 +4,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{cmp::max, collections::HashMap, convert::TryFrom, mem};
+use std::{cmp::max, collections::HashMap, mem};
 
 use neqo_common::{event::Provider, qdebug};
 use test_fixture::now;
diff --git a/third_party/rust/neqo-transport/src/connection/tests/vn.rs b/third_party/rust/neqo-transport/src/connection/tests/vn.rs
index 22f15c991c..93872a94f4 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/vn.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/vn.rs
@@ -7,7 +7,7 @@
 use std::{mem, time::Duration};
 
 use neqo_common::{event::Provider, Decoder, Encoder};
-use test_fixture::{self, assertions, datagram, now};
+use test_fixture::{assertions, datagram, now};
 
 use super::{
     super::{ConnectionError, ConnectionEvent, Output, State, ZeroRttState},
diff --git a/third_party/rust/neqo-transport/src/connection/tests/zerortt.rs b/third_party/rust/neqo-transport/src/connection/tests/zerortt.rs
index 0aa5573c98..b5e5f0d758 100644
--- a/third_party/rust/neqo-transport/src/connection/tests/zerortt.rs
+++ b/third_party/rust/neqo-transport/src/connection/tests/zerortt.rs
@@ -8,7 +8,7 @@ use std::{cell::RefCell, rc::Rc};
 
 use neqo_common::event::Provider;
 use neqo_crypto::{AllowZeroRtt, AntiReplay};
-use test_fixture::{self, assertions, now};
+use test_fixture::{assertions, now};
 
 use super::{
     super::Connection, connect, default_client, default_server, exchange_ticket, new_server,