Merge branch 'main' into chore-unwrap-less

Signed-off-by: Lars Eggert <lars@eggert.org>
mozilla · Jan 10, 2025 · 189d79f · 189d79f
2 parents 4ddddd7 + e664280
commit 189d79f
Show file tree

Hide file tree

Showing 15 changed files with 59 additions and 54 deletions.
diff --git a/.github/actions/rust/action.yml b/.github/actions/rust/action.yml
@@ -24,8 +24,7 @@ runs:
     - name: Install Rust
       uses: dtolnay/rust-toolchain@21dc36fb71dd22e3317045c0c31a3f4249868b17 # master
       with:
-        # TODO: Unpin once https://github.com/rust-lang/rust/issues/135235 is fixed.
-        toolchain: ${{ inputs.version == 'nightly' && 'nightly-2025-01-07' || inputs.version }}
+        toolchain: ${{ inputs.version  }}
         components: ${{ inputs.components }}
         targets: ${{ inputs.targets }}
 

diff --git a/Cargo.toml b/Cargo.toml
@@ -59,6 +59,9 @@ unused_macro_rules = "warn"
 cargo = { level = "warn", priority = -1 }
 nursery = { level = "warn", priority = -1 }
 pedantic = { level = "warn", priority = -1 }
+cfg_not_test = "warn"
+clone_on_ref_ptr = "warn"
+create_dir = "warn"
 if_then_some_else_none = "warn"
 get_unwrap = "warn"
 multiple_inherent_impl = "warn"
@@ -69,6 +72,7 @@ renamed_function_params = "warn"
 semicolon_inside_block = "warn"
 try_err = "warn"
 unneeded_field_pattern = "warn"
+unused_result_ok = "warn"
 unused_trait_names = "warn"
 unwrap_used = "warn"
 unwrap_in_result = "warn"

diff --git a/neqo-bin/src/lib.rs b/neqo-bin/src/lib.rs
@@ -283,7 +283,7 @@ mod tests {
                     .unwrap()
                     .as_secs()
             ));
-            fs::create_dir(&dir).unwrap();
+            fs::create_dir_all(&dir).unwrap();
             Self { path: dir }
         }
 

diff --git a/neqo-http3/src/connection.rs b/neqo-http3/src/connection.rs
@@ -736,8 +736,12 @@ impl Http3Connection {
                     conn.stream_stop_sending(stream_id, Error::HttpStreamCreation.code())?;
                     return Ok(ReceiveOutput::NoOutput);
                 }
-                // set incoming WebTransport streams to be fair (share bandwidth)
-                conn.stream_fairness(stream_id, true).ok();
+                // Set incoming WebTransport streams to be fair (share bandwidth).
+                // We may call this with an invalid stream ID, so ignore that error.
+                match conn.stream_fairness(stream_id, true) {
+                    Ok(()) | Err(neqo_transport::Error::InvalidStreamId) => (),
+                    Err(e) => return Err(Error::from(e)),
+                };
                 qinfo!(
                     [self],
                     "A new WebTransport stream {} for session {}.",
@@ -901,7 +905,7 @@ impl Http3Connection {
             MessageType::Request,
             stream_type,
             stream_id,
-            self.qpack_encoder.clone(),
+            Rc::clone(&self.qpack_encoder),
             send_events,
         );
 
@@ -1132,8 +1136,8 @@ impl Http3Connection {
         )));
         self.add_streams(
             id,
-            Box::new(extended_conn.clone()),
-            Box::new(extended_conn.clone()),
+            Box::new(Rc::clone(&extended_conn)),
+            Box::new(Rc::clone(&extended_conn)),
         );
 
         let final_headers = Self::create_fetch_headers(&RequestDescription {
@@ -1220,7 +1224,7 @@ impl Http3Connection {
                         )?));
                     self.add_streams(
                         stream_id,
-                        Box::new(extended_conn.clone()),
+                        Box::new(Rc::clone(&extended_conn)),
                         Box::new(extended_conn),
                     );
                     self.streams_with_pending_data.insert(stream_id);
@@ -1286,7 +1290,6 @@ impl Http3Connection {
             .stream_create(stream_type)
             .map_err(|e| Error::map_stream_create_errors(&e))?;
         // Set outgoing WebTransport streams to be fair (share bandwidth)
-        // This really can't fail, panics if it does
         conn.stream_fairness(stream_id, true)?;
 
         self.webtransport_create_stream_internal(
@@ -1371,7 +1374,7 @@ impl Http3Connection {
                     stream_id,
                     session_id,
                     send_events,
-                    webtransport_session.clone(),
+                    Rc::clone(&webtransport_session),
                     local,
                 )),
                 Box::new(WebTransportRecvStream::new(

diff --git a/neqo-http3/src/connection_client.rs b/neqo-http3/src/connection_client.rs
@@ -813,10 +813,6 @@ impl Http3Client {
     /// # Errors
     ///
     /// It may return `InvalidStreamId` if a stream does not exist anymore.
-    ///
-    /// # Panics
-    ///
-    /// This cannot panic.
     pub fn webtransport_set_fairness(&mut self, stream_id: StreamId, fairness: bool) -> Res<()> {
         Http3Connection::stream_set_fairness(&mut self.conn, stream_id, fairness)
     }

diff --git a/neqo-http3/src/connection_server.rs b/neqo-http3/src/connection_server.rs
@@ -325,7 +325,7 @@ impl Http3ServerHandler {
                         MessageType::Response,
                         Http3StreamType::Http,
                         stream_id,
-                        self.base_handler.qpack_encoder.clone(),
+                        Rc::clone(&self.base_handler.qpack_encoder),
                         Box::new(self.events.clone()),
                     )),
                     Box::new(RecvMessage::new(

diff --git a/neqo-http3/src/features/extended_connect/webtransport_session.rs b/neqo-http3/src/features/extended_connect/webtransport_session.rs
@@ -73,7 +73,7 @@ impl WebTransportSession {
                     first_frame_type: None,
                 },
                 qpack_decoder,
-                Box::new(stream_event_listener.clone()),
+                Box::new(Rc::clone(&stream_event_listener)),
                 None,
                 PriorityHandler::new(false, Priority::default()),
             )),
@@ -82,7 +82,7 @@ impl WebTransportSession {
                 Http3StreamType::ExtendedConnect,
                 session_id,
                 qpack_encoder,
-                Box::new(stream_event_listener.clone()),
+                Box::new(Rc::clone(&stream_event_listener)),
             )),
             stream_event_listener,
             session_id,
@@ -110,11 +110,11 @@ impl WebTransportSession {
         control_stream_recv
             .http_stream()
             .ok_or(Error::Internal)?
-            .set_new_listener(Box::new(stream_event_listener.clone()));
+            .set_new_listener(Box::new(Rc::clone(&stream_event_listener)));
         control_stream_send
             .http_stream()
             .ok_or(Error::Internal)?
-            .set_new_listener(Box::new(stream_event_listener.clone()));
+            .set_new_listener(Box::new(Rc::clone(&stream_event_listener)));
         Ok(Self {
             control_stream_recv,
             control_stream_send,
@@ -451,7 +451,7 @@ impl RecvStream for Rc<RefCell<WebTransportSession>> {
     }
 
     fn webtransport(&self) -> Option<Rc<RefCell<WebTransportSession>>> {
-        Some(self.clone())
+        Some(Self::clone(self))
     }
 }
 

diff --git a/neqo-http3/src/server.rs b/neqo-http3/src/server.rs
@@ -151,6 +151,7 @@ impl Http3Server {
         }
     }
 
+    #[allow(clippy::too_many_lines)]
     fn process_events(&mut self, conn: &ConnectionRef, now: Instant) {
         let mut remove = false;
         let http3_parameters = &self.http3_parameters;
@@ -171,7 +172,11 @@ impl Http3Server {
                         headers,
                         fin,
                     } => self.events.headers(
-                        Http3OrWebTransportStream::new(conn.clone(), handler.clone(), stream_info),
+                        Http3OrWebTransportStream::new(
+                            conn.clone(),
+                            Rc::clone(handler),
+                            stream_info,
+                        ),
                         headers,
                         fin,
                     ),
@@ -187,15 +192,19 @@ impl Http3Server {
                     }
                     Http3ServerConnEvent::DataWritable { stream_info } => self
                         .events
-                        .data_writable(conn.clone(), handler.clone(), stream_info),
+                        .data_writable(conn.clone(), Rc::clone(handler), stream_info),
                     Http3ServerConnEvent::StreamReset { stream_info, error } => {
-                        self.events
-                            .stream_reset(conn.clone(), handler.clone(), stream_info, error);
+                        self.events.stream_reset(
+                            conn.clone(),
+                            Rc::clone(handler),
+                            stream_info,
+                            error,
+                        );
                     }
                     Http3ServerConnEvent::StreamStopSending { stream_info, error } => {
                         self.events.stream_stop_sending(
                             conn.clone(),
-                            handler.clone(),
+                            Rc::clone(handler),
                             stream_info,
                             error,
                         );
@@ -215,7 +224,7 @@ impl Http3Server {
                     }
                     Http3ServerConnEvent::ExtendedConnect { stream_id, headers } => {
                         self.events.webtransport_new_session(
-                            WebTransportRequest::new(conn.clone(), handler.clone(), stream_id),
+                            WebTransportRequest::new(conn.clone(), Rc::clone(handler), stream_id),
                             headers,
                         );
                     }
@@ -225,22 +234,22 @@ impl Http3Server {
                         headers,
                         ..
                     } => self.events.webtransport_session_closed(
-                        WebTransportRequest::new(conn.clone(), handler.clone(), stream_id),
+                        WebTransportRequest::new(conn.clone(), Rc::clone(handler), stream_id),
                         reason,
                         headers,
                     ),
                     Http3ServerConnEvent::ExtendedConnectNewStream(stream_info) => self
                         .events
                         .webtransport_new_stream(Http3OrWebTransportStream::new(
                             conn.clone(),
-                            handler.clone(),
+                            Rc::clone(handler),
                             stream_info,
                         )),
                     Http3ServerConnEvent::ExtendedConnectDatagram {
                         session_id,
                         datagram,
                     } => self.events.webtransport_datagram(
-                        WebTransportRequest::new(conn.clone(), handler.clone(), session_id),
+                        WebTransportRequest::new(conn.clone(), Rc::clone(handler), session_id),
                         datagram,
                     ),
                 }
@@ -293,7 +302,7 @@ fn prepare_data(
                     data.resize(amount, 0);
                 }
 
-                events.data(conn.clone(), handler.clone(), stream_info, data, fin);
+                events.data(conn.clone(), Rc::clone(handler), stream_info, data, fin);
             }
             if amount < MAX_EVENT_DATA_SIZE || fin {
                 break;

diff --git a/neqo-http3/src/server_events.rs b/neqo-http3/src/server_events.rs
@@ -333,7 +333,7 @@ impl WebTransportRequest {
 
         Ok(Http3OrWebTransportStream::new(
             self.stream_handler.conn.clone(),
-            self.stream_handler.handler.clone(),
+            Rc::clone(&self.stream_handler.handler),
             Http3StreamInfo::new(id, Http3StreamType::WebTransport(session_id)),
         ))
     }

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
@@ -55,10 +55,7 @@ use crate::{
     stats::{Stats, StatsCell},
     stream_id::StreamType,
     streams::{SendOrder, Streams},
-    tparams::{
-        self, TransportParameter, TransportParameterId, TransportParameters,
-        TransportParametersHandler,
-    },
+    tparams::{self, TransportParameters, TransportParametersHandler},
     tracking::{AckTracker, PacketNumberSpace, RecvdPackets},
     version::{Version, WireVersion},
     AppError, CloseReason, Error, Res, StreamId,
@@ -406,7 +403,7 @@ impl Connection {
             state: State::Init,
             paths: Paths::default(),
             cid_manager,
-            tps: tphandler.clone(),
+            tps: Rc::clone(&tphandler),
             zero_rtt_state: ZeroRttState::Init,
             address_validation: AddressValidationInfo::None,
             local_initial_source_cid,
@@ -444,7 +441,7 @@ impl Connection {
         zero_rtt_checker: impl ZeroRttChecker + 'static,
     ) -> Res<()> {
         self.crypto
-            .server_enable_0rtt(self.tps.clone(), anti_replay, zero_rtt_checker)
+            .server_enable_0rtt(Rc::clone(&self.tps), anti_replay, zero_rtt_checker)
     }
 
     /// # Errors
@@ -499,11 +496,12 @@ impl Connection {
     /// 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))]
-        {
-            assert!(!tparams::INTERNAL_TRANSPORT_PARAMETERS.contains(&tp));
-        }
+    #[cfg(test)]
+    pub fn set_local_tparam(
+        &self,
+        tp: tparams::TransportParameterId,
+        value: tparams::TransportParameter,
+    ) -> Res<()> {
         if *self.state() == State::Init {
             self.tps.borrow_mut().local.set(tp, value);
             Ok(())
@@ -1558,7 +1556,7 @@ impl Connection {
         );
         path.borrow_mut().add_received(d.len());
         let res = self.input_path(&path, d, received);
-        self.capture_error(Some(path), now, 0, res).ok();
+        _ = self.capture_error(Some(path), now, 0, res);
     }
 
     fn input_path(

diff --git a/neqo-transport/src/connection/tests/stream.rs b/neqo-transport/src/connection/tests/stream.rs
@@ -131,8 +131,8 @@ fn sendorder_test(order_of_sendorder: &[Option<SendOrder>]) {
         streams.push(id);
         ordered.push((id, *sendorder));
         // must be set before sendorder
-        client.streams.set_fairness(id, true).ok();
-        client.streams.set_sendorder(id, *sendorder).ok();
+        client.streams.set_fairness(id, true).unwrap();
+        client.streams.set_sendorder(id, *sendorder).unwrap();
     }
     // Write some data to all the streams
     for stream_id in streams {

diff --git a/neqo-transport/src/tparams.rs b/neqo-transport/src/tparams.rs
@@ -31,10 +31,6 @@ pub type TransportParameterId = u64;
 macro_rules! tpids {
         { $($n:ident = $v:expr),+ $(,)? } => {
             $(pub const $n: TransportParameterId = $v;)+
-
-            /// A complete list of internal transport parameters.
-            #[cfg(not(test))]
-            pub(crate) const INTERNAL_TRANSPORT_PARAMETERS: &[TransportParameterId] = &[ $($n),+ ];
         };
     }
 tpids! {

diff --git a/neqo-transport/tests/connection.rs b/neqo-transport/tests/connection.rs
@@ -284,8 +284,8 @@ fn handshake_mlkem768x25519() {
 
     client
         .set_groups(&[neqo_crypto::TLS_GRP_KEM_MLKEM768X25519])
-        .ok();
-    client.send_additional_key_shares(0).ok();
+        .unwrap();
+    client.send_additional_key_shares(0).unwrap();
 
     test_fixture::handshake(&mut client, &mut server);
     assert_eq!(*client.state(), State::Confirmed);

diff --git a/neqo-udp/src/lib.rs b/neqo-udp/src/lib.rs
@@ -233,7 +233,7 @@ mod tests {
         // platforms. Use `std` socket instead.  See also
         // <https://github.com/quinn-rs/quinn/pull/2123>.
         let sender = std::net::UdpSocket::bind("127.0.0.1:0")?;
-        let receiver = Socket::new(std::net::UdpSocket::bind("127.0.0.1:0")?)?;
+        let receiver = socket()?;
         let receiver_addr: SocketAddr = "127.0.0.1:0".parse().unwrap();
 
         sender.send_to(&[], receiver.inner.local_addr()?)?;

diff --git a/test-fixture/src/sim/mod.rs b/test-fixture/src/sim/mod.rs
@@ -251,7 +251,7 @@ impl Simulator {
 
         qinfo!("{}: seed {}", self.name, self.rng.borrow().seed_str());
         for n in &mut self.nodes {
-            n.init(self.rng.clone(), start);
+            n.init(Rc::clone(&self.rng), start);
         }
 
         let setup_start = Instant::now();