fix(udp): ignore empty datagram (mozilla#2044)

* test(udp): assert ignoring of empty datagram

* fix(udp): ignore empty datagram

When receiving an emtpy datagram `meta.len` and `meta.stride` would be `0`.
Chunking the receive buffer via `.chunks(0)` would panic with:

```
chunk size must be non-zero
```

See also panic docs on `slice::chunks`:

https://doc.rust-lang.org/std/primitive.slice.html#method.chunks

With this commit `recv_inner` ignores the empty datagram.

In addition, under the assumption that an empty datagram is not a faulty event,
`recv_inner` attempts another receive call on the socket. This ensures that the
socket eventually returns `WouldBlock` and is thus registered for the next
wake-up with the corresponding event loop (e.g. tokio`).

neqo-bin/src/udp.rs

@@ -57,7 +57,7 @@ impl Socket {
     pub fn recv(&self, local_address: &SocketAddr) -> Result<Vec<Datagram>, io::Error> {
         self.inner
             .try_io(tokio::io::Interest::READABLE, || {
-                neqo_udp::recv_inner(local_address, &self.state, (&self.inner).into())
+                neqo_udp::recv_inner(local_address, &self.state, &self.inner)
             })
             .or_else(|e| {
                 if e.kind() == io::ErrorKind::WouldBlock {

neqo-udp/src/lib.rs

@@ -13,7 +13,7 @@ use std::{
     slice,
 };
 
-use neqo_common::{qtrace, Datagram, IpTos};
+use neqo_common::{qdebug, qtrace, Datagram, IpTos};
 use quinn_udp::{EcnCodepoint, RecvMeta, Transmit, UdpSocketState};
 
 /// Socket receive buffer size.
@@ -52,22 +52,44 @@ pub fn send_inner(
     Ok(())
 }
 
+#[cfg(unix)]
+use std::os::fd::AsFd as SocketRef;
+#[cfg(windows)]
+use std::os::windows::io::AsSocket as SocketRef;
+
 pub fn recv_inner(
     local_address: &SocketAddr,
     state: &UdpSocketState,
-    socket: quinn_udp::UdpSockRef<'_>,
+    socket: impl SocketRef,
 ) -> Result<Vec<Datagram>, io::Error> {
     let dgrams = RECV_BUF.with_borrow_mut(|recv_buf| -> Result<Vec<Datagram>, io::Error> {
-        let mut meta = RecvMeta::default();
+        let mut meta;
+
+        loop {
+            meta = RecvMeta::default();
+
+            state.recv(
+                (&socket).into(),
+                &mut [IoSliceMut::new(recv_buf)],
+                slice::from_mut(&mut meta),
+            )?;
+
+            if meta.len == 0 || meta.stride == 0 {
+                qdebug!(
+                    "ignoring datagram from {} to {} len {} stride {}",
+                    meta.addr,
+                    local_address,
+                    meta.len,
+                    meta.stride
+                );
+                continue;
+            }
 
-        state.recv(
-            socket,
-            &mut [IoSliceMut::new(recv_buf)],
-            slice::from_mut(&mut meta),
-        )?;
+            break;
+        }
 
         Ok(recv_buf[0..meta.len]
-            .chunks(meta.stride.min(recv_buf.len()))
+            .chunks(meta.stride)
             .map(|d| {
                 qtrace!(
                     "received {} bytes from {} to {}",
@@ -100,9 +122,7 @@ pub struct Socket<S> {
     inner: S,
 }
 
-impl<#[cfg(unix)] S: std::os::fd::AsFd, #[cfg(windows)] S: std::os::windows::io::AsSocket>
-    Socket<S>
-{
+impl<S: SocketRef> Socket<S> {
     /// Create a new [`Socket`] given a raw file descriptor managed externally.
     pub fn new(socket: S) -> Result<Self, io::Error> {
         Ok(Self {
@@ -119,7 +139,7 @@ impl<#[cfg(unix)] S: std::os::fd::AsFd, #[cfg(windows)] S: std::os::windows::io:
     /// Receive a batch of [`Datagram`]s on the given [`Socket`], each
     /// set with the provided local address.
     pub fn recv(&self, local_address: &SocketAddr) -> Result<Vec<Datagram>, io::Error> {
-        recv_inner(local_address, &self.state, (&self.inner).into())
+        recv_inner(local_address, &self.state, &self.inner)
     }
 }
 
@@ -136,6 +156,26 @@ mod tests {
         Ok(socket)
     }
 
+    #[test]
+    fn ignore_empty_datagram() -> Result<(), io::Error> {
+        let sender = socket()?;
+        let receiver = Socket::new(std::net::UdpSocket::bind("127.0.0.1:0")?)?;
+        let receiver_addr: SocketAddr = "127.0.0.1:0".parse().unwrap();
+
+        let datagram = Datagram::new(
+            sender.inner.local_addr()?,
+            receiver.inner.local_addr()?,
+            IpTos::default(),
+            vec![],
+        );
+
+        sender.send(&datagram)?;
+        let res = receiver.recv(&receiver_addr);
+        assert_eq!(res.unwrap_err().kind(), std::io::ErrorKind::WouldBlock);
+
+        Ok(())
+    }
+
     #[test]
     fn datagram_tos() -> Result<(), io::Error> {
         let sender = socket()?;