Add unit tests for hostname parsing (#715)

Add unit tests for subdomain parsing

Author: paulgb

plane/src/proxy/mod.rs

@@ -19,6 +19,7 @@ mod proxy_service;
 mod rewriter;
 mod route_map;
 mod shutdown_signal;
+mod subdomain;
 mod tls;
 
 #[derive(Debug, Clone, Copy)]

plane/src/proxy/rewriter.rs

@@ -1,4 +1,4 @@
-use super::ForwardableRequestInfo;
+use super::{subdomain::subdomain_from_host, ForwardableRequestInfo};
 use crate::{
     protocol::RouteInfo,
     types::{BearerToken, ClusterName},
@@ -21,7 +21,7 @@ const BACKEND_ID_HEADER: &str = "x-verified-backend";
 const X_FORWARDED_FOR_HEADER: &str = "x-forwarded-for";
 const X_FORWARDED_PROTO_HEADER: &str = "x-forwarded-proto";
 
-#[derive(Debug, thiserror::Error)]
+#[derive(Debug, thiserror::Error, PartialEq, Eq)]
 pub enum RequestRewriterError {
     #[error("Invalid `host` header")]
     InvalidHostHeader,
@@ -105,36 +105,7 @@ impl RequestRewriter {
             }
         };
 
-        // Remove port from hostname if present and port is 443.
-        // We are already a bit magic with regards to HTTP/HTTPS: we assume that if a
-        // cluster name has a colon, it's HTTP and runs on that port, and if a cluster
-        // name does not have a colon, it is HTTPS and runs on 443. In other words,
-        // in production, there's no way for URLs generated and sent to clients to have
-        // a port other than the standard HTTPS port 443.
-        let hostname = if let Some(hostname) = hostname.strip_suffix(":443") {
-            hostname
-        } else {
-            hostname
-        };
-
-        let Some(subdomain) = hostname.strip_suffix(cluster.as_str()) else {
-            tracing::warn!(hostname, "Host header does not end in cluster name.");
-            return Err(RequestRewriterError::InvalidHostHeader);
-        };
-
-        if subdomain.is_empty() {
-            return Ok(None);
-        }
-
-        let subdomain = match subdomain.strip_suffix('.') {
-            Some(subdomain) => subdomain,
-            None => {
-                tracing::warn!(hostname, "Host header does not start with a dot.");
-                return Err(RequestRewriterError::InvalidHostHeader);
-            }
-        };
-
-        Ok(Some(subdomain))
+        subdomain_from_host(hostname, cluster)
     }
 
     fn into_parts(self) -> (request::Parts, Body, Uri, ForwardableRequestInfo) {

plane/src/proxy/subdomain.rs

@@ -0,0 +1,110 @@
+use super::rewriter::RequestRewriterError;
+use crate::types::ClusterName;
+
+// If a cluster name does not specify a port, :443 is implied.
+// Most browsers will not specify it, but some (e.g. the `ws` websocket client in Node.js)
+// will, so we strip it.
+const HTTPS_PORT_SUFFIX: &str = ":443";
+
+/// Returns Ok(Some(subdomain)) if a subdomain is found.
+/// Returns Ok(None) if no subdomain is found, but the host header matches the cluster name.
+/// Returns Err(RequestRewriterError::InvalidHostHeader) if the host header does not
+/// match the cluster name.
+pub fn subdomain_from_host<'a>(
+    host: &'a str,
+    cluster: &ClusterName,
+) -> Result<Option<&'a str>, RequestRewriterError> {
+    let host = if let Some(host) = host.strip_suffix(HTTPS_PORT_SUFFIX) {
+        host
+    } else {
+        host
+    };
+
+    if let Some(subdomain) = host.strip_suffix(cluster.as_str()) {
+        if subdomain.is_empty() {
+            // Subdomain exactly matches cluster name.
+            Ok(None)
+        } else if let Some(subdomain) = subdomain.strip_suffix('.') {
+            Ok(Some(subdomain))
+        } else {
+            Err(RequestRewriterError::InvalidHostHeader)
+        }
+    } else {
+        tracing::warn!(host, "Host header does not end in cluster name.");
+        Err(RequestRewriterError::InvalidHostHeader)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::str::FromStr;
+
+    #[test]
+    fn no_subdomains() {
+        let host = "foo.bar.baz";
+        let cluster = ClusterName::from_str("foo.bar.baz").unwrap();
+        assert_eq!(subdomain_from_host(host, &cluster), Ok(None));
+    }
+
+    #[test]
+    fn valid_subdomain() {
+        let host = "foobar.example.com";
+        let cluster = ClusterName::from_str("example.com").unwrap();
+        assert_eq!(subdomain_from_host(host, &cluster), Ok(Some("foobar")));
+    }
+
+    #[test]
+    fn valid_suffix_no_dot() {
+        let host = "foobarexample.com";
+        let cluster = ClusterName::from_str("example.com").unwrap();
+        assert_eq!(
+            subdomain_from_host(host, &cluster),
+            Err(RequestRewriterError::InvalidHostHeader)
+        );
+    }
+
+    #[test]
+    fn invalid_suffix() {
+        let host = "abc.abc.com";
+        let cluster = ClusterName::from_str("example.com").unwrap();
+        assert_eq!(
+            subdomain_from_host(host, &cluster),
+            Err(RequestRewriterError::InvalidHostHeader)
+        );
+    }
+
+    #[test]
+    fn allowed_port() {
+        let host = "foobar.myhost:8080";
+        let cluster = ClusterName::from_str("myhost:8080").unwrap();
+        assert_eq!(subdomain_from_host(host, &cluster), Ok(Some("foobar")));
+    }
+
+    #[test]
+    fn port_required() {
+        let host = "foobar.myhost";
+        let cluster = ClusterName::from_str("myhost:8080").unwrap();
+        assert_eq!(
+            subdomain_from_host(host, &cluster),
+            Err(RequestRewriterError::InvalidHostHeader)
+        );
+    }
+
+    #[test]
+    fn port_must_match() {
+        let host = "foobar.myhost:8080";
+        let cluster = ClusterName::from_str("myhost").unwrap();
+        assert_eq!(
+            subdomain_from_host(host, &cluster),
+            Err(RequestRewriterError::InvalidHostHeader)
+        );
+    }
+
+    #[test]
+    fn port_443_optional() {
+        let host = "foobar.myhost:443";
+        let cluster = ClusterName::from_str("myhost").unwrap();
+        assert_eq!(subdomain_from_host(host, &cluster), Ok(Some("foobar")));
+    }
+}