Repository: bearcove/loona (language: rust)

Issue:
Set `uri` in `Request` struct
h2 sends us everything we need:

```
2023-01-21T18:10:51.046209Z DEBUG hring::h2::server: HEADER | :method: GET
2023-01-21T18:10:51.046218Z DEBUG hring::h2::server: HEADER | :scheme: http
2023-01-21T18:10:51.046230Z DEBUG hring::h2::server: ignoring pseudo-header
2023-01-21T18:10:51.046235Z DEBUG hring::h2::server: HEADER | :path: /
2023-01-21T18:10:51.046478Z DEBUG hring::h2::server: HEADER | :authority: 127.0.0.1:8888
```

h1 a little less:

```
2023-01-21T18:11:05.748766Z DEBUG hring::h1::server: src/h1/server.rs:79: got request Request { method: POST, path: "/echo-body", version: HTTP/1.1 }
```

(host headers would show up here if we had any).

There's no real equivalent to `scheme` in HTTP/1.1, unfortunately, although we could pass that knowledge from somewhere else, potentially.. I'm not sure what hyper does here.


Discussion / hints:

Fix patch:
diff --git a/crates/hring-bingo-ktls/src/main.rs b/crates/hring-bingo-ktls/src/main.rs
index 4165c94f..267059ee 100644
--- a/crates/hring-bingo-ktls/src/main.rs
+++ b/crates/hring-bingo-ktls/src/main.rs
@@ -126,7 +126,7 @@ impl ServerDriver for SDriver {
         req_body: &mut impl Body,
         respond: Responder<E, ExpectResponseHeaders>,
     ) -> eyre::Result<Responder<E, ResponseDone>> {
-        info!("Handling {:?} {}", req.method, req.path);
+        info!("Handling {:?} {}", req.method, req.uri);
 
         let addr = "httpbingo.org:80"
             .to_socket_addrs()?
diff --git a/crates/hring/src/buffet/piece.rs b/crates/hring/src/buffet/piece.rs
index 1c35b9d9..dfad7e19 100644
--- a/crates/hring/src/buffet/piece.rs
+++ b/crates/hring/src/buffet/piece.rs
@@ -78,8 +78,8 @@ impl Piece {
         std::str::from_utf8(self.as_ref())
     }
 
-    /// Decode as utf-8
-    pub fn to_string(self) -> Result<PieceStr, Utf8Error> {
+    /// Decode as utf-8 (owned)
+    pub fn to_str(self) -> Result<PieceStr, Utf8Error> {
         _ = std::str::from_utf8(&self)?;
         Ok(PieceStr { piece: self })
     }
diff --git a/crates/hring/src/h1/encode.rs b/crates/hring/src/h1/encode.rs
index c2a5d66d..fd38e8e5 100644
--- a/crates/hring/src/h1/encode.rs
+++ b/crates/hring/src/h1/encode.rs
@@ -15,13 +15,16 @@ use super::body::{write_h1_body_chunk, write_h1_body_end, BodyWriteMode};
 pub(crate) fn encode_request(req: Request, list: &mut PieceList) -> eyre::Result<()> {
     list.push(req.method.into_chunk());
     list.push(" ");
-    list.push(req.path);
+    // TODO: get rid of allocation here, or at least go through `RollMut`
+    list.push(req.uri.path().to_string().into_bytes());
     match req.version {
         Version::HTTP_10 => list.push(" HTTP/1.0\r\n"),
         Version::HTTP_11 => list.push(" HTTP/1.1\r\n"),
         _ => return Err(eyre::eyre!("unsupported HTTP version {:?}", req.version)),
     }
 
+    // TODO: if `host` isn't set, set from request uri? which should
+    // take precedence here?
     encode_headers(req.headers, list)?;
     list.push("\r\n");
     Ok(())
diff --git a/crates/hring/src/h1/parse.rs b/crates/hring/src/h1/parse.rs
index 6862f529..f9efb257 100644
--- a/crates/hring/src/h1/parse.rs
+++ b/crates/hring/src/h1/parse.rs
@@ -37,7 +37,9 @@ pub fn request(i: Roll) -> IResult<Roll, Request> {
 
     let request = Request {
         method,
-        path: path.into(),
+        // TODO: should this take the host header into account?
+        // check what hyper does.
+        uri: path.parse().unwrap(),
         version,
         headers,
     };
diff --git a/crates/hring/src/h2/server.rs b/crates/hring/src/h2/server.rs
index c9ed9ec3..88f6fbcd 100644
--- a/crates/hring/src/h2/server.rs
+++ b/crates/hring/src/h2/server.rs
@@ -1,7 +1,11 @@
 use std::{borrow::Cow, cell::RefCell, collections::HashMap, rc::Rc};
 
 use enumflags2::BitFlags;
-use http::{header::HeaderName, Version};
+use http::{
+    header::{self, HeaderName},
+    uri::{Authority, PathAndQuery, Scheme},
+    Version,
+};
 use nom::Finish;
 use smallvec::{smallvec, SmallVec};
 use tokio::sync::mpsc;
@@ -607,8 +611,11 @@ fn end_headers(
     driver: &Rc<impl ServerDriver + 'static>,
     hpack_dec: &mut hring_hpack::Decoder,
 ) -> eyre::Result<StreamRxStage> {
-    let mut path: Option<PieceStr> = None;
     let mut method: Option<Method> = None;
+    let mut scheme: Option<Scheme> = None;
+    let mut path: Option<PieceStr> = None;
+    let mut authority: Option<Authority> = None;
+
     let mut headers = Headers::default();
 
     let cb = |key: Cow<[u8]>, value: Cow<[u8]>| {
@@ -620,15 +627,28 @@ fn end_headers(
 
         if &key[..1] == b":" {
             // it's a pseudo-header!
+            // TODO: reject headers that occur after pseudo-headers
+            // TODO: reject duplicate pseudo-headers
             match &key[1..] {
+                b"method" => {
+                    // TODO: error handling
+                    let value: PieceStr = Piece::from(value.to_vec()).to_str().unwrap();
+                    method = Some(Method::try_from(value).unwrap());
+                }
+                b"scheme" => {
+                    // TODO: error handling
+                    let value: PieceStr = Piece::from(value.to_vec()).to_str().unwrap();
+                    scheme = Some(value.parse().unwrap());
+                }
                 b"path" => {
-                    let value: PieceStr = Piece::from(value.to_vec()).to_string().unwrap();
+                    // TODO: error handling
+                    let value: PieceStr = Piece::from(value.to_vec()).to_str().unwrap();
                     path = Some(value);
                 }
-                b"method" => {
+                b"authority" => {
                     // TODO: error handling
-                    let value: PieceStr = Piece::from(value.to_vec()).to_string().unwrap();
-                    method = Some(Method::try_from(value).unwrap());
+                    let value: PieceStr = Piece::from(value.to_vec()).to_str().unwrap();
+                    authority = Some(value.parse().unwrap());
                 }
                 _ => {
                     debug!("ignoring pseudo-header");
@@ -665,13 +685,35 @@ fn end_headers(
         }
     };
 
+    // TODO: cf. https://httpwg.org/specs/rfc9113.html#HttpRequest
+    // A server SHOULD treat a request as malformed if it contains a Host header
+    // field that identifies an entity that differs from the entity in the
+    // ":authority" pseudo-header field.
+
     // TODO: proper error handling (return 400)
-    let path = path.unwrap();
     let method = method.unwrap();
+    let scheme = scheme.unwrap();
+
+    let path = path.unwrap();
+    let path_and_query: PathAndQuery = path.parse().unwrap();
+
+    let authority = match authority {
+        Some(authority) => Some(authority),
+        None => headers
+            .get(header::HOST)
+            .map(|host| host.as_str().unwrap().parse().unwrap()),
+    };
+
+    let mut uri_parts: http::uri::Parts = Default::default();
+    uri_parts.scheme = Some(scheme);
+    uri_parts.authority = authority;
+    uri_parts.path_and_query = Some(path_and_query);
+
+    let uri = http::uri::Uri::from_parts(uri_parts).unwrap();
 
     let req = Request {
         method,
-        path,
+        uri,
         version: Version::HTTP_2,
         headers,
     };
diff --git a/crates/hring/src/types/mod.rs b/crates/hring/src/types/mod.rs
index e51cbc19..8d3614d0 100644
--- a/crates/hring/src/types/mod.rs
+++ b/crates/hring/src/types/mod.rs
@@ -1,9 +1,9 @@
 use std::fmt::{self, Debug};
 
-use http::{StatusCode, Version};
+use http::{StatusCode, Uri, Version};
 use tracing::debug;
 
-use crate::{Piece, PieceStr};
+use crate::Piece;
 
 mod headers;
 pub use headers::*;
@@ -17,8 +17,7 @@ pub struct Request {
     pub method: Method,
 
     /// Requested entity
-    /// TODO: use `Uri` here, this isn't http2-friendly
-    pub path: PieceStr,
+    pub uri: Uri,
 
     /// The HTTP version used
     pub version: Version,
@@ -31,7 +30,7 @@ impl Default for Request {
     fn default() -> Self {
         Self {
             method: Method::Get,
-            path: "/".into(),
+            uri: "/".parse().unwrap(),
             version: Version::HTTP_11,
             headers: Default::default(),
         }
@@ -44,7 +43,7 @@ impl fmt::Debug for Request {
 
         f.debug_struct("Request")
             .field("method", &self.method)
-            .field("path", &self.path)
+            .field("uri", &self.uri)
             .field("version", &self.version)
             .finish()?;
 


Test patch:
diff --git a/crates/hring/tests/integration_test.rs b/crates/hring/tests/integration_test.rs
index 4ce29801..5dd581cb 100644
--- a/crates/hring/tests/integration_test.rs
+++ b/crates/hring/tests/integration_test.rs
@@ -141,7 +141,7 @@ fn request_api() {
 
         let req = Request {
             method: Method::Get,
-            path: "/".into(),
+            uri: "/".parse().unwrap(),
             ..Default::default()
         };
 
diff --git a/test-crates/hring-h2spec/src/main.rs b/test-crates/hring-h2spec/src/main.rs
index 9b65efe6..3511a23a 100644
--- a/test-crates/hring-h2spec/src/main.rs
+++ b/test-crates/hring-h2spec/src/main.rs
@@ -46,9 +46,9 @@ impl ServerDriver for SDriver {
         respond: Responder<E, ExpectResponseHeaders>,
     ) -> color_eyre::Result<Responder<E, ResponseDone>> {
         tracing::info!(
-            "Handling {:?} {:?}, content_len = {:?}",
+            "Handling {:?} {}, content_len = {:?}",
             req.method,
-            req.path,
+            req.uri,
             req_body.content_len()
         );