quinn-rs#2009: Address code review feedback.

quinn-proto/src/connection/mod.rs

@@ -317,10 +317,10 @@ impl Connection {
             next_crypto: None,
             accepted_0rtt: false,
             permit_idle_reset: true,
-            idle_timeout: config
-                .max_idle_timeout
-                .filter(|dur| dur.0 != 0)
-                .map(|dur| Duration::from_millis(dur.0)),
+            idle_timeout: match config.max_idle_timeout {
+                None | Some(VarInt(0)) => None,
+                Some(dur) => Some(Duration::from_millis(dur.0)),
+            },
             timers: TimerTable::default(),
             authentication_failures: 0,
             error: None,
@@ -3293,10 +3293,8 @@ impl Connection {
 
     fn set_peer_params(&mut self, params: TransportParameters) {
         self.streams.set_params(&params);
-        self.idle_timeout = negotiate_max_idle_timeout(
-            self.config.max_idle_timeout.unwrap_or_default(),
-            params.max_idle_timeout,
-        );
+        self.idle_timeout =
+            negotiate_max_idle_timeout(self.config.max_idle_timeout, Some(params.max_idle_timeout));
         trace!("negotiated max idle timeout {:?}", self.idle_timeout);
         if let Some(ref info) = params.preferred_address {
             self.rem_cids.insert(frame::NewConnectionId {
@@ -3777,12 +3775,19 @@ impl SentFrames {
     }
 }
 
-fn negotiate_max_idle_timeout(x: VarInt, y: VarInt) -> Option<Duration> {
+/// Compute the negotiated idle timeout based on local and remote max_idle_timeout transport parameters.
+///
+/// According to the definition of max_idle_timeout, a value of `0` means the timeout is disabled; see <https://www.rfc-editor.org/rfc/rfc9000#section-18.2-4.4.1.>
+///
+/// According to the negotiation procedure, either the minimum of the timeouts or one specified is used as the negotiated value; see <https://www.rfc-editor.org/rfc/rfc9000#section-10.1-2.>
+///
+/// Returns the negotiated idle timeout as a `Duration`, or `None` when both endpoints have opted out of idle timeout.
+fn negotiate_max_idle_timeout(x: Option<VarInt>, y: Option<VarInt>) -> Option<Duration> {
     match (x, y) {
-        (VarInt(0), VarInt(0)) => None,
-        (VarInt(0), y) => Some(Duration::from_millis(y.0)),
-        (x, VarInt(0)) => Some(Duration::from_millis(x.0)),
-        (x, y) => Some(Duration::from_millis(cmp::min(x, y).0)),
+        (Some(VarInt(0)) | None, Some(VarInt(0)) | None) => None,
+        (Some(VarInt(0)) | None, Some(y)) => Some(Duration::from_millis(y.0)),
+        (Some(x), Some(VarInt(0)) | None) => Some(Duration::from_millis(x.0)),
+        (Some(x), Some(y)) => Some(Duration::from_millis(cmp::min(x, y).0)),
     }
 }
 
@@ -3793,10 +3798,20 @@ mod tests {
     #[test]
     fn negotiate_max_idle_timeout_commutative() {
         let test_params = [
-            (VarInt(0), VarInt(0), None),
-            (VarInt(2), VarInt(0), Some(Duration::from_millis(2))),
-            (VarInt(0), VarInt(3), Some(Duration::from_millis(3))),
-            (VarInt(1), VarInt(4), Some(Duration::from_millis(1))),
+            (None, None, None),
+            (None, Some(VarInt(0)), None),
+            (None, Some(VarInt(2)), Some(Duration::from_millis(2))),
+            (Some(VarInt(0)), Some(VarInt(0)), None),
+            (
+                Some(VarInt(2)),
+                Some(VarInt(0)),
+                Some(Duration::from_millis(2)),
+            ),
+            (
+                Some(VarInt(1)),
+                Some(VarInt(4)),
+                Some(Duration::from_millis(1)),
+            ),
         ];
 
         for (left, right, result) in test_params {