Repository: bearcove/loona (language: rust)

Issue:
h2: Support (and ignore, for now) priority frames
https://lib.rs/crates/h2 doesn't do anything with priorities right now - PRIORITY frames are one of the ways to set priorities for streams (the other is the PRIORITY flag in HEADERS).
h2: Support (and ignore, for now) priority frames
https://lib.rs/crates/h2 doesn't do anything with priorities right now - PRIORITY frames are one of the ways to set priorities for streams (the other is the PRIORITY flag in HEADERS).


Discussion / hints:

Fix patch:
diff --git a/.github/labeler.yml b/.github/labeler.yml
new file mode 100644
index 00000000..9c0f910d
--- /dev/null
+++ b/.github/labeler.yml
@@ -0,0 +1,10 @@
+h2:
+  - 'h2:'
+h1:
+  - 'h1:'
+perf:
+  - 'perf:'
+bug:
+  - 'bug:'
+feat:
+  - 'feat:'
diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml
new file mode 100644
index 00000000..68fb29f3
--- /dev/null
+++ b/.github/workflows/labeler.yml
@@ -0,0 +1,17 @@
+name: "Issue Labeler"
+on:
+  issues:
+    types: [opened, edited]
+  pull_request:
+    types: [opened, edited]
+
+jobs:
+  triage:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: github/issue-labeler@v2.5
+      with:
+        repo-token: "${{ secrets.GITHUB_TOKEN }}"
+        configuration-path: .github/labeler.yml
+        not-before: 2020-01-15T02:54:32Z
+        enable-versioned-regex: 0
\ No newline at end of file
diff --git a/.github/workflows/report.yml b/.github/workflows/report.yml
index 7c407a06..394b1f34 100644
--- a/.github/workflows/report.yml
+++ b/.github/workflows/report.yml
@@ -22,4 +22,4 @@ jobs:
         uses: mikepenz/action-junit-report@v3
         with:
           commit: ${{github.event.workflow_run.head_sha}}
-          report_paths: 'target/h2spec-junit'
\ No newline at end of file
+          report_paths: 'target/h2spec-junit.xml'
\ No newline at end of file
diff --git a/src/h2/encode.rs b/src/h2/encode.rs
index 279e9273..ea028cb2 100644
--- a/src/h2/encode.rs
+++ b/src/h2/encode.rs
@@ -4,13 +4,13 @@ use http::{StatusCode, Version};
 use tokio::sync::mpsc;
 use tracing::warn;
 
-use crate::{h1::body::BodyWriteMode, Encoder, Piece, Response};
+use crate::{h1::body::BodyWriteMode, Encoder, Piece, Response, Roll};
 
 use super::parse::{Frame, StreamId};
 
 pub(crate) enum H2ConnEvent {
-    Ping(Piece),
-    ClientFrame(Frame, Piece),
+    Ping(Roll),
+    ClientFrame(Frame, Option<Roll>),
     ServerEvent(H2Event),
     AcknowledgeSettings,
 }
diff --git a/src/h2/mod.rs b/src/h2/mod.rs
index bd3d33ab..21eb3f42 100644
--- a/src/h2/mod.rs
+++ b/src/h2/mod.rs
@@ -4,6 +4,8 @@
 mod server;
 pub use server::*;
 
+pub(crate) mod parse;
+
 mod encode;
 
 mod body;
diff --git a/src/h2/server/parse.rs b/src/h2/parse.rs
similarity index 95%
rename from src/h2/server/parse.rs
rename to src/h2/parse.rs
index 46d9b0e8..79d3646d 100644
--- a/src/h2/server/parse.rs
+++ b/src/h2/parse.rs
@@ -275,20 +275,22 @@ fn parse_reserved_and_stream_id(i: Roll) -> IResult<Roll, (u8, StreamId)> {
 
 // cf. https://httpwg.org/specs/rfc9113.html#HEADERS
 #[derive(Debug)]
-pub(crate) struct HeadersPriority {
+pub(crate) struct PrioritySpec {
     pub exclusive: bool,
     pub stream_dependency: StreamId,
     // 0-255 => 1-256
     pub weight: u8,
 }
 
-pub(crate) fn parse_headers_priority(i: Roll) -> IResult<Roll, HeadersPriority> {
-    map(
-        tuple((parse_reserved_and_stream_id, be_u8)),
-        |((exclusive, stream_dependency), weight)| HeadersPriority {
-            exclusive: exclusive != 0,
-            stream_dependency,
-            weight,
-        },
-    )(i)
+impl PrioritySpec {
+    pub(crate) fn parse(i: Roll) -> IResult<Roll, Self> {
+        map(
+            tuple((parse_reserved_and_stream_id, be_u8)),
+            |((exclusive, stream_dependency), weight)| Self {
+                exclusive: exclusive != 0,
+                stream_dependency,
+                weight,
+            },
+        )(i)
+    }
 }
diff --git a/src/h2/server.rs b/src/h2/server.rs
index a840c706..00e1dfdc 100644
--- a/src/h2/server.rs
+++ b/src/h2/server.rs
@@ -12,13 +12,13 @@ use crate::{
         body::H2Body,
         encode::{EncoderState, H2ConnEvent, H2Encoder, H2EventPayload},
         parse::{
-            parse_headers_priority, DataFlags, Frame, FrameType, HeadersFlags, PingFlags,
+            self, DataFlags, Frame, FrameType, HeadersFlags, PingFlags, PrioritySpec,
             SettingsFlags, StreamId,
         },
     },
     util::read_and_parse,
     ExpectResponseHeaders, Headers, Method, Piece, PieceStr, ReadWriteOwned, Request, Responder,
-    RollMut, ServerDriver,
+    Roll, RollMut, ServerDriver,
 };
 
 /// HTTP/2 server configuration
@@ -32,8 +32,6 @@ impl Default for ServerConf {
     }
 }
 
-pub(crate) mod parse;
-
 #[derive(Default)]
 struct ConnState {
     streams: HashMap<StreamId, StreamState>,
@@ -127,8 +125,8 @@ async fn h2_read_loop(
         // TODO: there might be optimizations to be done for `Data` frames later
         // on, but for now, let's unconditionally read the payload (if it's not
         // empty).
-        let payload = if frame.len == 0 {
-            Piece::Static(&[])
+        let payload: Option<Roll> = if frame.len == 0 {
+            None
         } else {
             let payload_roll;
             (client_buf, payload_roll) = match read_and_parse(
@@ -149,7 +147,7 @@ async fn h2_read_loop(
                 }
             };
 
-            payload_roll.into()
+            Some(payload_roll)
         };
 
         match frame.frame_type {
@@ -178,7 +176,19 @@ async fn h2_read_loop(
                     return Err(eyre::eyre!("could not send H2 event"));
                 }
             }
-            FrameType::Priority => todo!(),
+            FrameType::Priority => {
+                let payload = if let Some(payload) = payload {
+                    payload
+                } else {
+                    todo!("handle connection error: priority frame with no payload");
+                };
+
+                let pri_spec = match PrioritySpec::parse(payload) {
+                    Ok((_rest, pri_spec)) => pri_spec,
+                    Err(e) => todo!("handle connection error: invalid priority frame {e}"),
+                };
+                debug!(?pri_spec, "received priority frame");
+            }
             FrameType::RstStream => todo!(),
             FrameType::Settings(s) => {
                 if s.contains(SettingsFlags::Ack) {
@@ -208,7 +218,14 @@ async fn h2_read_loop(
                     continue;
                 }
 
-                if ev_tx.send(H2ConnEvent::Ping(payload.into())).await.is_err() {
+                let payload = match payload {
+                    Some(payload) => payload,
+                    None => {
+                        todo!("handle connection error: ping frame with no payload");
+                    }
+                };
+
+                if ev_tx.send(H2ConnEvent::Ping(payload)).await.is_err() {
                     return Err(eyre::eyre!("could not send H2 ping event"));
                 }
             }
@@ -255,20 +272,21 @@ async fn h2_write_loop(
                     }
 
                     if flags.contains(HeadersFlags::Priority) {
-                        let roll = match payload {
-                            Piece::Roll(roll) => roll,
-                            Piece::Static(_) => unreachable!(),
-                            Piece::Vec(_) => unreachable!(),
-                            Piece::HeaderName(_) => unreachable!(),
+                        let pri_spec;
+
+                        // TODO: proper error handling (connection error probably)
+                        let payload_in = match payload {
+                            Some(payload) => payload,
+                            None => {
+                                todo!("handle connection error: priority flag set, but no payload");
+                            }
                         };
-                        let roll_out;
-                        let prio;
-                        // TODO: proper error handling (stream error probably)
-                        (roll_out, prio) = parse_headers_priority(roll)
+                        let payload_out;
+                        (payload_out, pri_spec) = PrioritySpec::parse(payload_in)
                             .finish()
                             .map_err(|err| eyre::eyre!("parsing error: {err:?}"))?;
-                        payload = Piece::Roll(roll_out);
-                        debug!(exclusive = %prio.exclusive, stream_dependency = ?prio.stream_dependency, weight = %prio.weight, "received priority, exclusive");
+                        debug!(exclusive = %pri_spec.exclusive, stream_dependency = ?pri_spec.stream_dependency, weight = %pri_spec.weight, "received priority, exclusive");
+                        payload = Some(payload_out);
                     }
 
                     let mut path: Option<PieceStr> = None;
@@ -277,42 +295,47 @@ async fn h2_write_loop(
                     let mut headers = Headers::default();
                     debug!("receiving headers for stream {}", frame.stream_id);
 
-                    hpack_dec
-                        .decode_with_cb(&payload[..], |key, value| {
-                            debug!(
-                                "got key: {:?}\nvalue: {:?}",
-                                key.hex_dump(),
-                                value.hex_dump()
-                            );
-
-                            if &key[..1] == b":" {
-                                // it's a pseudo-header!
-                                match &key[1..] {
-                                    b"path" => {
-                                        let value: PieceStr =
-                                            Piece::from(value.to_vec()).to_string().unwrap();
-                                        path = Some(value);
-                                    }
-                                    b"method" => {
-                                        // TODO: error handling
-                                        let value: PieceStr =
-                                            Piece::from(value.to_vec()).to_string().unwrap();
-                                        method = Some(Method::try_from(value).unwrap());
-                                    }
-                                    other => {
-                                        debug!("ignoring pseudo-header {:?}", other.hex_dump());
+                    if let Some(payload) = payload {
+                        hpack_dec
+                            .decode_with_cb(&payload[..], |key, value| {
+                                debug!(
+                                    "got key: {:?}\nvalue: {:?}",
+                                    key.hex_dump(),
+                                    value.hex_dump()
+                                );
+
+                                if &key[..1] == b":" {
+                                    // it's a pseudo-header!
+                                    match &key[1..] {
+                                        b"path" => {
+                                            let value: PieceStr =
+                                                Piece::from(value.to_vec()).to_string().unwrap();
+                                            path = Some(value);
+                                        }
+                                        b"method" => {
+                                            // TODO: error handling
+                                            let value: PieceStr =
+                                                Piece::from(value.to_vec()).to_string().unwrap();
+                                            method = Some(Method::try_from(value).unwrap());
+                                        }
+                                        other => {
+                                            debug!("ignoring pseudo-header {:?}", other.hex_dump());
+                                        }
                                     }
+                                } else {
+                                    // TODO: what do we do in case of malformed header names?
+                                    // ignore it? return a 400?
+                                    let name = HeaderName::from_bytes(&key[..])
+                                        .expect("malformed header name");
+                                    let value: Piece = value.to_vec().into();
+                                    headers.append(name, value);
                                 }
-                            } else {
-                                // TODO: what do we do in case of malformed header names?
-                                // ignore it? return a 400?
-                                let name = HeaderName::from_bytes(&key[..])
-                                    .expect("malformed header name");
-                                let value: Piece = value.to_vec().into();
-                                headers.append(name, value);
-                            }
-                        })
-                        .map_err(|e| eyre::eyre!("hpack error: {e:?}"))?;
+                            })
+                            .map_err(|e| eyre::eyre!("hpack error: {e:?}"))?;
+
+                        // TODO: hpack errors are COMPRESSION_ERROR, cf.
+                        // https://httpwg.org/specs/rfc9113.html#ErrorCodes
+                    }
 
                     // TODO: proper error handling (return 400)
                     let path = path.unwrap();
@@ -390,7 +413,7 @@ async fn h2_write_loop(
                         todo!("padded data is not supported");
                     }
 
-                    let tx = {
+                    let body_tx = {
                         let mut state = state.borrow_mut();
                         let stream = state
                             .streams
@@ -416,8 +439,11 @@ async fn h2_write_loop(
                             }
                         }
                     };
-                    if tx.send(Ok(payload)).await.is_err() {
-                        return Err(eyre::eyre!("ev_tx receiver dropped"));
+
+                    if let Some(payload) = payload {
+                        if body_tx.send(Ok(payload.into())).await.is_err() {
+                            return Err(eyre::eyre!("ev_tx receiver dropped"));
+                        }
                     }
                 }
                 _ => {


Test patch: