fix pathological MAX_DATA/STREAM_MAX_DATA update behavior

quiche/src/flowcontrol.rs

@@ -59,7 +59,7 @@ impl FlowControl {
         Self {
             max_data,
 
-            window,
+            window: std::cmp::min(window, max_window),
 
             max_window,
 
@@ -137,6 +137,13 @@ impl FlowControl {
 mod tests {
     use super::*;
 
+    #[test]
+    fn max_window_in_new() {
+        let fc = FlowControl::new(100, 100, 50);
+        assert_eq!(fc.max_data(), 100);
+        assert_eq!(fc.window, 50);
+    }
+
     #[test]
     fn max_data() {
         let fc = FlowControl::new(100, 20, 100);
@@ -223,5 +230,9 @@ mod tests {
         // Window changed to the new value.
         fc.ensure_window_lower_bound(w * 2);
         assert_eq!(fc.window(), 40);
+
+        // Window clamped to max_window
+        fc.ensure_window_lower_bound(101);
+        assert_eq!(fc.window(), 100);
     }
 }

quiche/src/lib.rs

@@ -480,9 +480,6 @@ const MAX_UNDECRYPTABLE_PACKETS: usize = 10;
 
 const RESERVED_VERSION_MASK: u32 = 0xfafafafa;
 
-// The default size of the receiver connection flow control window.
-const DEFAULT_CONNECTION_WINDOW: u64 = 48 * 1024;
-
 // The maximum size of the receiver connection flow control window.
 const MAX_CONNECTION_WINDOW: u64 = 24 * 1024 * 1024;
 
@@ -2008,7 +2005,7 @@ impl<F: BufFactory> Connection<F> {
             rx_data: 0,
             flow_control: flowcontrol::FlowControl::new(
                 max_rx_data,
-                cmp::min(max_rx_data / 2 * 3, DEFAULT_CONNECTION_WINDOW),
+                max_rx_data,
                 config.max_connection_window,
             ),
             should_send_max_data: false,

quiche/src/stream/mod.rs

@@ -46,9 +46,6 @@ use crate::Result;
 
 const DEFAULT_URGENCY: u8 = 127;
 
-// The default size of the receiver stream flow control window.
-const DEFAULT_STREAM_WINDOW: u64 = 32 * 1024;
-
 /// The maximum size of the receiver stream flow control window.
 pub const MAX_STREAM_WINDOW: u64 = 16 * 1024 * 1024;
 
@@ -945,6 +942,9 @@ impl ExactSizeIterator for StreamIter {
     }
 }
 
+#[cfg(test)]
+const DEFAULT_STREAM_WINDOW: u64 = 32 * 1024;
+
 #[cfg(test)]
 mod tests {
     use crate::range_buf::RangeBuf;

quiche/src/stream/recv_buf.rs

@@ -41,8 +41,6 @@ use crate::flowcontrol;
 
 use crate::range_buf::RangeBuf;
 
-use super::DEFAULT_STREAM_WINDOW;
-
 /// Receive-side stream buffer.
 ///
 /// Stream data received by the peer is buffered in a list of data chunks
@@ -78,9 +76,7 @@ impl RecvBuf {
     pub fn new(max_data: u64, max_window: u64) -> RecvBuf {
         RecvBuf {
             flow_control: flowcontrol::FlowControl::new(
-                max_data,
-                cmp::min(max_data, DEFAULT_STREAM_WINDOW),
-                max_window,
+                max_data, max_data, max_window,
             ),
             ..RecvBuf::default()
         }
@@ -416,6 +412,7 @@ impl RecvBuf {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::stream::DEFAULT_STREAM_WINDOW;
     use rstest::rstest;
 
     // Helper function for testing either buffer emit or discard.

quiche/src/tests.rs

@@ -1422,6 +1422,9 @@ fn flow_control_update(
     let mut pipe = test_utils::Pipe::new(cc_algorithm_name).unwrap();
     assert_eq!(pipe.handshake(), Ok(()));
 
+    // Make sure the pipe is configured as we expect
+    assert_eq!(pipe.server.max_rx_data(), 30);
+
     let frames = [
         frame::Frame::Stream {
             stream_id: 0,
@@ -1465,7 +1468,9 @@ fn flow_control_update(
             max: 30
         })
     );
-    assert_eq!(iter.next(), Some(&frame::Frame::MaxData { max: 61 }));
+    // Initial max_data/window was 30, we consumed/read 16 bytes, which is more
+    // than 1/2 the window ==> new max data is 30 + 16
+    assert_eq!(iter.next(), Some(&frame::Frame::MaxData { max: 46 }));
 }
 
 #[rstest]
@@ -3293,7 +3298,7 @@ fn stream_shutdown_read_after_fin(
     assert_eq!(pipe.handshake(), Ok(()));
 
     // Client sends some data and a FIN.
-    assert_eq!(pipe.client.stream_send(4, b"hello, world", true), Ok(12));
+    assert_eq!(pipe.client.stream_send(4, b"hello, world123", true), Ok(15));
     assert_eq!(pipe.advance(), Ok(()));
 
     let mut r = pipe.server.readable();
@@ -3309,17 +3314,9 @@ fn stream_shutdown_read_after_fin(
     let mut r = pipe.server.readable();
     assert_eq!(r.next(), None);
 
-    // Server sends a flow control update, but it does NOT send
-    // STOP_SENDING frame, since it has already received a FIN from
-    // the client.
-    let (len, _) = pipe.server.send(&mut buf).unwrap();
-    let mut dummy = buf[..len].to_vec();
-    let frames =
-        test_utils::decode_pkt(&mut pipe.client, &mut dummy[..len]).unwrap();
-    for f in frames {
-        assert!(!matches!(f, frame::Frame::StopSending { .. }));
-    }
-    assert_eq!(pipe.client_recv(&mut buf[..len]), Ok(len));
+    // Server does NOT send STOP_SENDING frame, since it has already received a
+    // FIN from the client.
+    assert_eq!(pipe.server.send(&mut buf), Err(Error::Done));
 
     assert_eq!(pipe.advance(), Ok(()));
 
@@ -3424,14 +3421,14 @@ fn stream_shutdown_read_update_max_data(
 
     // The client has dropped the 9 unset bytes in its buffer
     assert_eq!(pipe.client.tx_data, 21);
+    // The 21 consumed bytes have been added on the client
+    assert_eq!(pipe.client.max_tx_data, 30 + 21);
+    // ... and the client can send again
+    assert_eq!(pipe.client.stream_send(4, &[0], false), Ok(1));
+
+    // Server side is unchanged
     assert_eq!(pipe.server.rx_data, 21);
     assert_eq!(pipe.server.flow_control.consumed(), 21);
-    // default window is 1.5 * initial_max_data, so 45
-    assert_eq!(
-        pipe.client.tx_cap,
-        pipe.server.flow_control.window() as usize
-    );
-    assert_eq!(pipe.client.tx_cap, 45);
 
     assert_eq!(
         pipe.client.stream_send(0, b"hello, world", false),
@@ -3440,8 +3437,8 @@ fn stream_shutdown_read_update_max_data(
 
     // fully advance pipe
     assert_eq!(pipe.advance(), Ok(()));
-    assert_eq!(pipe.client.tx_data, 21);
-    assert_eq!(pipe.server.rx_data, 21);
+    assert_eq!(pipe.client.tx_data, 22);
+    assert_eq!(pipe.server.rx_data, 22);
     assert!(!pipe.server.stream_readable(0)); // nothing can be consumed
 
     // Server sends fin to fully close the stream.
@@ -3513,8 +3510,8 @@ fn stream_shutdown_write_update_max_data(
     assert_eq!(pipe.server.rx_data, 30);
     assert_eq!(pipe.server.flow_control.consumed(), 30);
     assert_eq!(pipe.client.tx_data, 30);
-    // default window is 1.5 * initial_max_data, so 45
-    assert_eq!(pipe.client.tx_cap, 45);
+    // new max_tx_data is old_max_tx_data + consumed == 30 + 30
+    assert_eq!(pipe.client.max_tx_data, 60);
 
     // client can send again on a different stream
     assert_eq!(pipe.client.stream_send(4, b"a", false), Ok(1));