Merge pull request #90 from fastly/aturon/uri-and-host-semantics

Make URI and Host header semantics more explicit
fastly · Nov 11, 2021 · b85d0f5 · b85d0f5
2 parents 0d776fc + 29a79de
commit b85d0f5
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 43 deletions.
diff --git a/lib/src/upstream.rs b/lib/src/upstream.rs
@@ -3,10 +3,11 @@ use crate::{
     wiggle_abi::types::PendingRequestHandle,
 };
 use futures::Future;
-use http::uri;
-use hyper::{client::HttpConnector, Client, Request, Response, Uri};
+use http::{uri, HeaderValue};
+use hyper::{client::HttpConnector, Client, HeaderMap, Request, Response, Uri};
 use hyper_rustls::{HttpsConnector, MaybeHttpsStream};
 use std::pin::Pin;
+use std::str::FromStr;
 use std::task::{self, Poll};
 use tokio::{net::TcpStream, sync::oneshot};
 
@@ -49,6 +50,65 @@ impl hyper::service::Service<Uri> for Connector {
     }
 }
 
+fn canonical_host_header(
+    original_headers: &HeaderMap,
+    original_uri: &Uri,
+    backend: &Backend,
+) -> HeaderValue {
+    backend
+        .override_host
+        .clone()
+        .or_else(|| original_headers.get(hyper::header::HOST).cloned())
+        .or_else(|| {
+            original_uri
+                .authority()
+                .and_then(|auth| HeaderValue::from_str(auth.as_str()).ok())
+        })
+        .expect("Could determine a Host header")
+}
+
+fn canonical_uri(original_uri: &Uri, canonical_host: &str, backend: &Backend) -> Uri {
+    let original_path = original_uri
+        .path_and_query()
+        .map_or("/", uri::PathAndQuery::as_str);
+
+    let mut canonical_uri = String::new();
+
+    // Hyper's `Client` API _requires_ a URI in "absolute form", including scheme, authority,
+    // and path.
+
+    // We start with the scheme, taken from the backend (which determines what we're actually
+    // communicating over).
+    canonical_uri.push_str(
+        backend
+            .uri
+            .scheme_str()
+            .expect("Backend URL included a scheme"),
+    );
+    canonical_uri.push_str("://");
+
+    // We get the authority from the canonical host. In some cases that might actually come
+    // from the `original_uri`, but usually it's from an explicit `Host` header.
+    canonical_uri.push_str(canonical_host);
+
+    // The path begins with the "path prefix" present in the backend's URI. This is often just
+    // an empty path or `/`.
+    canonical_uri.push_str(backend.uri.path());
+    if !canonical_uri.ends_with('/') {
+        canonical_uri.push('/');
+    }
+
+    // Finally we incorporate the requested path, taking care not to introduce extra `/`
+    // separators when gluing things together.
+    if let Some(stripped) = original_path.strip_prefix('/') {
+        canonical_uri.push_str(stripped)
+    } else {
+        canonical_uri.push_str(original_path)
+    }
+
+    Uri::from_str(&canonical_uri).expect("URI could be parsed")
+}
+
 /// Sends the given request to the given backend.
 ///
 /// Note that the backend's URI is used to determine which host to route the request to; the URI
@@ -57,53 +117,32 @@ impl hyper::service::Service<Uri> for Connector {
 pub fn send_request(
     mut req: Request<Body>,
     backend: &Backend,
-) -> Result<impl Future<Output = Result<Response<Body>, Error>>, Error> {
+) -> impl Future<Output = Result<Response<Body>, Error>> {
     let connector = Connector::new(backend);
 
-    // stitch on the backend path prefix, if it has one
-    if backend.uri.path() != "/" {
-        // first, we have to fully break apart the request into parts, to get access to the path component
-        let (mut req_parts, req_body) = req.into_parts();
-        let mut uri_parts = req_parts.uri.into_parts();
-        let path_and_query = uri_parts
-            .path_and_query
-            .as_ref()
-            .map_or("", uri::PathAndQuery::as_str);
-
-        // build up a prefixed path, taking care to ensure there's exactly one `/` separator between the
-        // backend path prefix and the request's URL path
-        let mut prefixed_path = backend.uri.path().to_owned();
-        if !prefixed_path.ends_with('/') {
-            prefixed_path.push('/');
-        }
-        if let Some(stripped) = path_and_query.strip_prefix('/') {
-            prefixed_path.push_str(stripped)
-        } else {
-            prefixed_path.push_str(path_and_query)
-        };
-
-        // now stitch back up the request
-        uri_parts.path_and_query =
-            Some(prefixed_path.parse().expect("Prefixed URI failed to parse"));
-        req_parts.uri = Uri::from_parts(uri_parts).expect("Prefixed URI failed to parse");
-        req = Request::from_parts(req_parts, req_body);
-    }
-
-    // if requested override the host header
-    if let Some(override_host) = &backend.override_host {
-        req.headers_mut()
-            .insert(hyper::header::HOST, override_host.clone());
-    }
+    let host = canonical_host_header(req.headers(), req.uri(), backend);
+    let uri = canonical_uri(
+        req.uri(),
+        std::str::from_utf8(host.as_bytes()).expect("Host was in UTF-8"),
+        backend,
+    );
 
     filter_outgoing_headers(req.headers_mut());
+    req.headers_mut().insert(hyper::header::HOST, host);
+    *req.uri_mut() = uri;
 
-    Ok(async move {
+    async move {
         Ok(Client::builder()
+            .set_host(false)
             .build(connector)
             .request(req)
-            .await?
+            .await
+            .map_err(|e| {
+                eprintln!("Error: {:?}", e);
+                e
+            })?
             .map(Body::from))
-    })
+    }
 }
 
 /// The type ultimately yielded by a `PendingRequest`.

diff --git a/lib/src/wiggle_abi/req_impl.rs b/lib/src/wiggle_abi/req_impl.rs
@@ -346,7 +346,7 @@ impl FastlyHttpReq for Session {
             .ok_or_else(|| Error::UnknownBackend(backend_name.to_owned()))?;
 
         // synchronously send the request
-        let resp = upstream::send_request(req, backend)?.await?;
+        let resp = upstream::send_request(req, backend).await?;
         Ok(self.insert_response(resp))
     }
 
@@ -368,7 +368,7 @@ impl FastlyHttpReq for Session {
             .ok_or_else(|| Error::UnknownBackend(backend_name.to_owned()))?;
 
         // asynchronously send the request
-        let pending_req = PendingRequest::spawn(upstream::send_request(req, backend)?);
+        let pending_req = PendingRequest::spawn(upstream::send_request(req, backend));
 
         // return a handle to the pending request
         Ok(self.insert_pending_request(pending_req))
@@ -392,7 +392,7 @@ impl FastlyHttpReq for Session {
             .ok_or_else(|| Error::UnknownBackend(backend_name.to_owned()))?;
 
         // asynchronously send the request
-        let pending_req = PendingRequest::spawn(upstream::send_request(req, backend)?);
+        let pending_req = PendingRequest::spawn(upstream::send_request(req, backend));
 
         // return a handle to the pending request
         Ok(self.insert_pending_request(pending_req))