http: Use the endpoint type to inform URI normalization (#404)

Currently, the `http::normalize_uri` middleware determines absolute-form
authorities by reading request extensions. This hidden coupling makes
changing things fairly difficult. Instead, we can use the target types,
which already hold the relevant info, to determine the absolute-form
authority if needed.

Author: olix0r

linkerd/addr/src/lib.rs

@@ -74,10 +74,14 @@ impl Addr {
         }
     }
 
-    pub fn as_authority(&self) -> http::uri::Authority {
+    pub fn to_http_authority(&self) -> http::uri::Authority {
         match self {
-            Addr::Name(n) => n.as_authority(),
-            Addr::Socket(a) => http::uri::Authority::from_str(&format!("{}", a))
+            Addr::Name(n) => n.as_http_authority(),
+            Addr::Socket(ref a) if a.port() == 80 => {
+                http::uri::Authority::from_str(&a.ip().to_string())
+                    .expect("SocketAddr must be valid authority")
+            }
+            Addr::Socket(a) => http::uri::Authority::from_str(&a.to_string())
                 .expect("SocketAddr must be valid authority"),
         }
     }
@@ -113,6 +117,12 @@ impl fmt::Display for Addr {
     }
 }
 
+impl From<SocketAddr> for Addr {
+    fn from(sa: SocketAddr) -> Self {
+        Addr::Socket(sa)
+    }
+}
+
 impl From<NameAddr> for Addr {
     fn from(na: NameAddr) -> Self {
         Addr::Name(na)
@@ -175,9 +185,14 @@ impl NameAddr {
         self.name.is_localhost()
     }
 
-    pub fn as_authority(&self) -> http::uri::Authority {
-        http::uri::Authority::from_str(&format!("{}", self))
-            .expect("NameAddr must be valid authority")
+    pub fn as_http_authority(&self) -> http::uri::Authority {
+        if self.port == 80 {
+            http::uri::Authority::from_str(self.name.as_ref())
+                .expect("NameAddr must be valid authority")
+        } else {
+            http::uri::Authority::from_str(&self.to_string())
+                .expect("NameAddr must be valid authority")
+        }
     }
 }
 

linkerd/app/core/src/control.rs

@@ -88,7 +88,7 @@ pub mod add_origin {
         }
 
         fn call(&mut self, target: ControlAddr) -> Self::Future {
-            let authority = target.addr.as_authority();
+            let authority = target.addr.to_http_authority();
             let inner = self.inner.call(target);
             MakeFuture {
                 inner,

linkerd/app/inbound/src/endpoint.rs

@@ -1,13 +1,12 @@
-use http;
 use indexmap::IndexMap;
 use linkerd2_app_core::{
     classify,
     dst::{DstAddr, Route},
     metric_labels::EndpointLabels,
-    proxy::{http::settings, identity, tap},
+    proxy::{http, identity, tap},
     router,
     transport::{connect, tls},
-    Conditional, NameAddr,
+    Addr, Conditional, NameAddr,
 };
 use std::fmt;
 use std::net::SocketAddr;
@@ -18,7 +17,7 @@ use tracing::debug;
 pub struct Endpoint {
     pub addr: SocketAddr,
     pub dst_name: Option<NameAddr>,
-    pub http_settings: settings::Settings,
+    pub http_settings: http::Settings,
     pub tls_client_id: tls::PeerIdentity,
 }
 
@@ -34,7 +33,7 @@ impl From<SocketAddr> for Endpoint {
         Self {
             addr,
             dst_name: None,
-            http_settings: settings::Settings::NotHttp,
+            http_settings: http::Settings::NotHttp,
             tls_client_id: Conditional::None(tls::ReasonForNoPeerName::NotHttp.into()),
         }
     }
@@ -52,8 +51,26 @@ impl tls::HasPeerIdentity for Endpoint {
     }
 }
 
-impl settings::HasSettings for Endpoint {
-    fn http_settings(&self) -> &settings::Settings {
+impl http::normalize_uri::ShouldNormalizeUri for Endpoint {
+    fn should_normalize_uri(&self) -> Option<http::uri::Authority> {
+        if let http::Settings::Http1 {
+            was_absolute_form: false,
+            ..
+        } = self.http_settings
+        {
+            return Some(
+                self.dst_name
+                    .as_ref()
+                    .map(|dst| dst.as_http_authority())
+                    .unwrap_or_else(|| Addr::from(self.addr).to_http_authority()),
+            );
+        }
+        None
+    }
+}
+
+impl http::settings::HasSettings for Endpoint {
+    fn http_settings(&self) -> &http::Settings {
         &self.http_settings
     }
 }

linkerd/app/outbound/src/endpoint.rs

@@ -4,13 +4,13 @@ use linkerd2_app_core::{
     metric_labels::{prefix_labels, EndpointLabels},
     proxy::{
         api_resolve::{Metadata, ProtocolHint},
-        http::{identity_from_header, settings},
+        http::{self, identity_from_header},
         identity,
         resolve::map_endpoint::MapEndpoint,
         tap,
     },
     transport::{connect, tls},
-    Conditional, NameAddr, L5D_REQUIRE_ID,
+    Addr, Conditional, NameAddr, L5D_REQUIRE_ID,
 };
 use std::net::SocketAddr;
 use std::sync::Arc;
@@ -22,7 +22,7 @@ pub struct Endpoint {
     pub addr: SocketAddr,
     pub identity: tls::PeerIdentity,
     pub metadata: Metadata,
-    pub http_settings: settings::Settings,
+    pub http_settings: http::Settings,
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -36,13 +36,13 @@ impl Endpoint {
         }
 
         match self.http_settings {
-            settings::Settings::Http2 => false,
-            settings::Settings::Http1 {
+            http::Settings::Http2 => false,
+            http::Settings::Http1 {
                 keep_alive: _,
                 wants_h1_upgrade,
                 was_absolute_form: _,
             } => !wants_h1_upgrade,
-            settings::Settings::NotHttp => {
+            http::Settings::NotHttp => {
                 unreachable!(
                     "Endpoint::can_use_orig_proto called when NotHttp: {:?}",
                     self,
@@ -58,7 +58,7 @@ impl Endpoint {
             .addrs
             .target_addr_if_not_local()?;
 
-        let http_settings = settings::Settings::from_request(req);
+        let http_settings = http::Settings::from_request(req);
         let identity = match identity_from_header(req, L5D_REQUIRE_ID) {
             Some(require_id) => Conditional::Some(require_id),
             None => {
@@ -85,7 +85,7 @@ impl From<SocketAddr> for Endpoint {
             dst_concrete: None,
             identity: Conditional::None(tls::ReasonForNoPeerName::NotHttp.into()),
             metadata: Metadata::empty(),
-            http_settings: settings::Settings::NotHttp,
+            http_settings: http::Settings::NotHttp,
         }
     }
 }
@@ -119,8 +119,26 @@ impl connect::HasPeerAddr for Endpoint {
     }
 }
 
-impl settings::HasSettings for Endpoint {
-    fn http_settings(&self) -> &settings::Settings {
+impl http::normalize_uri::ShouldNormalizeUri for Endpoint {
+    fn should_normalize_uri(&self) -> Option<http::uri::Authority> {
+        if let http::Settings::Http1 {
+            was_absolute_form: false,
+            ..
+        } = self.http_settings
+        {
+            return Some(
+                self.dst_logical
+                    .as_ref()
+                    .map(|dst| dst.as_http_authority())
+                    .unwrap_or_else(|| Addr::from(self.addr).to_http_authority()),
+            );
+        }
+        None
+    }
+}
+
+impl http::settings::HasSettings for Endpoint {
+    fn http_settings(&self) -> &http::Settings {
         &self.http_settings
     }
 }

linkerd/proxy/http/src/h1.rs

@@ -1,10 +1,7 @@
 use super::upgrade::HttpConnect;
-use bytes::BytesMut;
 use http;
 use http::header::{CONNECTION, HOST, UPGRADE};
 use http::uri::{Authority, Parts, Scheme, Uri};
-use linkerd2_proxy_transport::tls;
-use std::fmt::Write;
 use std::mem;
 use tracing::{debug, trace};
 
@@ -19,24 +16,9 @@ pub fn normalize_our_view_of_uri<B>(req: &mut http::Request<B>) {
     );
 
     // try to parse the Host header
-    if let Some(auth) = authority_from_host(&req) {
-        trace!("using host header");
-        set_authority(req.uri_mut(), auth);
-        return;
-    }
-
-    // last resort is to use the so_original_dst
-    if let Some(addr) = req
-        .extensions()
-        .get::<tls::accept::Meta>()
-        .map(|s| s.addrs.target_addr())
-    {
-        trace!(target.addr = %addr, "using target address");
-        let mut bytes = BytesMut::with_capacity(31);
-        write!(&mut bytes, "{}", addr).expect("socket address display is under 31 bytes");
-        let auth =
-            Authority::from_shared(bytes.freeze()).expect("socket address is valid authority");
-        set_authority(req.uri_mut(), auth);
+    if let Some(host) = authority_from_host(&req) {
+        trace!(%host, "normalizing URI");
+        set_authority(req.uri_mut(), host);
         return;
     }
 
@@ -45,6 +27,7 @@ pub fn normalize_our_view_of_uri<B>(req: &mut http::Request<B>) {
 
 /// Convert any URI into its origin-form (relative path part only).
 pub fn set_origin_form(uri: &mut Uri) {
+    trace!(%uri, "Setting origin form");
     let mut parts = mem::replace(uri, Uri::default()).into_parts();
     parts.scheme = None;
     parts.authority = None;
@@ -56,7 +39,7 @@ pub fn authority_from_host<B>(req: &http::Request<B>) -> Option<Authority> {
     super::authority_from_header(req, HOST)
 }
 
-fn set_authority(uri: &mut http::Uri, auth: Authority) {
+pub fn set_authority(uri: &mut http::Uri, auth: Authority) {
     let mut parts = Parts::from(mem::replace(uri, Uri::default()));
 
     parts.authority = Some(auth);

linkerd/proxy/http/src/lib.rs

@@ -33,7 +33,7 @@ pub use self::{
     settings::Settings,
     version::Version,
 };
-pub use http::{Request, Response};
+pub use http::{header, uri, Request, Response};
 
 pub trait HasH2Reason {
     fn h2_reason(&self) -> Option<::h2::Reason>;