Repository: bearcove/loona (language: rust)

Issue:
h2: Don't read payload if frame length is zero
Since we unconditionally read the payload, we got "refusing to do empty read": https://github.com/hapsoc/hring/actions/runs/3837788493/jobs/6533488526#step:3:759
h2: Don't read payload if frame length is zero
Since we unconditionally read the payload, we got "refusing to do empty read": https://github.com/hapsoc/hring/actions/runs/3837788493/jobs/6533488526#step:3:759


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/report.yml b/.github/workflows/report.yml
index 0880bac6..7c407a06 100644
--- a/.github/workflows/report.yml
+++ b/.github/workflows/report.yml
@@ -2,7 +2,7 @@ name: report
 
 on:
   workflow_run:
-    workflows: [build]
+    workflows: [test]
     types: [completed]
     
 permissions:
diff --git a/Justfile b/Justfile
index ed0d7824..ca32479d 100644
--- a/Justfile
+++ b/Justfile
@@ -8,7 +8,7 @@ ci-test:
 	#!/bin/bash -eux
 	just build-testbed
 	cargo llvm-cov --no-report nextest
-	cargo llvm-cov --no-report run --manifest-path h2spec-server/Cargo.toml -- -j 'target/junit.xml'
+	cargo llvm-cov --no-report run --manifest-path h2spec-server/Cargo.toml -- -j 'target/h2spec-junit.xml'
 	cargo llvm-cov report --lcov --output-path coverage.lcov
 	codecov
 
@@ -31,7 +31,10 @@ bench *args:
 	RUST_BACKTRACE=1 cargo bench {{args}} -- --plotting-backend plotters
 
 h2spec *args:
+	#!/bin/bash -eux
 	echo "This requires h2spec to be installed: https://github.com/summerwind/h2spec"
+	export RUST_LOG="${RUST_LOG:-debug}"
+	export RUST_BACKTRACE=1
 	cargo run --manifest-path h2spec-server/Cargo.toml -- {{args}}
 
 check:
diff --git a/src/h2/server.rs b/src/h2/server.rs
index c3d1a833..a840c706 100644
--- a/src/h2/server.rs
+++ b/src/h2/server.rs
@@ -125,24 +125,31 @@ async fn h2_read_loop(
         debug!(?frame, "received h2 frame");
 
         // TODO: there might be optimizations to be done for `Data` frames later
-        // on, but for now, let's unconditionally read the payload
-        let payload;
-        (client_buf, payload) = match read_and_parse(
-            nom::bytes::streaming::take(frame.len as usize),
-            transport.as_ref(),
-            client_buf,
-            frame.len as usize,
-        )
-        .await?
-        {
-            Some((client_buf, frame)) => (client_buf, frame),
-            None => {
-                debug!(
-                    "h2 client closed connection while reading payload for {:?}",
-                    frame.frame_type
-                );
-                return Ok(());
-            }
+        // on, but for now, let's unconditionally read the payload (if it's not
+        // empty).
+        let payload = if frame.len == 0 {
+            Piece::Static(&[])
+        } else {
+            let payload_roll;
+            (client_buf, payload_roll) = match read_and_parse(
+                nom::bytes::streaming::take(frame.len as usize),
+                transport.as_ref(),
+                client_buf,
+                frame.len as usize,
+            )
+            .await?
+            {
+                Some((client_buf, payload)) => (client_buf, payload),
+                None => {
+                    debug!(
+                        "h2 client closed connection while reading payload for {:?}",
+                        frame.frame_type
+                    );
+                    return Ok(());
+                }
+            };
+
+            payload_roll.into()
         };
 
         match frame.frame_type {
@@ -151,11 +158,7 @@ async fn h2_read_loop(
                     todo!("handle padded data frame");
                 }
 
-                if (ev_tx
-                    .send(H2ConnEvent::ClientFrame(frame, payload.into()))
-                    .await)
-                    .is_err()
-                {
+                if (ev_tx.send(H2ConnEvent::ClientFrame(frame, payload)).await).is_err() {
                     return Err(eyre::eyre!("could not send H2 event"));
                 }
             }
@@ -171,11 +174,7 @@ async fn h2_read_loop(
                     }
                 }
 
-                if (ev_tx
-                    .send(H2ConnEvent::ClientFrame(frame, payload.into()))
-                    .await)
-                    .is_err()
-                {
+                if (ev_tx.send(H2ConnEvent::ClientFrame(frame, payload)).await).is_err() {
                     return Err(eyre::eyre!("could not send H2 event"));
                 }
             }
diff --git a/src/util.rs b/src/util.rs
index e5be3e1f..e82c639a 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -20,7 +20,11 @@ where
     Parser: Fn(Roll) -> IResult<Roll, Output>,
 {
     loop {
-        trace!("reading+parsing ({} bytes so far)", buf.len());
+        trace!(
+            "reading+parsing (buf.len={}, buf.cap={})",
+            buf.len(),
+            buf.cap()
+        );
         let filled = buf.filled();
 
         match parser(filled) {
@@ -43,6 +47,15 @@ where
                         return Err(SemanticError::BufferLimitReachedWhileParsing.into());
                     }
 
+                    if buf.cap() == 0 {
+                        trace!("buf had zero cap, growing");
+                        buf.grow()
+                    }
+                    trace!(
+                        "calling read_into, buf.cap={}, buf.len={} read_limit={read_limit}",
+                        buf.cap(),
+                        buf.len()
+                    );
                     (res, buf) = buf.read_into(read_limit, stream).await;
 
                     let n = res.wrap_err("reading request headers from downstream")?;


Test patch: