refactor(iroh-relay)!: Remove relay_endpoint config option & rename /derp route to /relay

iroh-net/src/relay/http.rs

@@ -12,6 +12,12 @@ pub(crate) const HTTP_UPGRADE_PROTOCOL: &str = "iroh derp http";
 pub(crate) const WEBSOCKET_UPGRADE_PROTOCOL: &str = "websocket";
 pub(crate) const SUPPORTED_WEBSOCKET_VERSION: &str = "13";
 
+/// The path under which the relay accepts relaying connections
+/// (over websockets and a custom upgrade protocol).
+pub const RELAY_HTTP_PATH: &str = "/relay";
+/// The path under which the relay allows doing latency queries for testing.
+pub const RELAY_PROBE_HTTP_PATH: &str = "/relay/probe";
+
 #[cfg(test)]
 mod tests {
     use super::*;

iroh-net/src/relay/http/client.rs

@@ -31,6 +31,7 @@ use crate::key::{PublicKey, SecretKey};
 use crate::relay::client::{ConnReader, ConnWriter};
 use crate::relay::codec::DerpCodec;
 use crate::relay::http::streams::{downcast_upgrade, MaybeTlsStream};
+use crate::relay::http::RELAY_HTTP_PATH;
 use crate::relay::RelayUrl;
 use crate::relay::{
     client::Client as RelayClient, client::ClientBuilder as RelayClientBuilder,
@@ -202,12 +203,14 @@ impl PingTracker {
 }
 
 /// Build a Client.
+#[derive(derive_more::Debug)]
 pub struct ClientBuilder {
     /// Default is false
     can_ack_pings: bool,
     /// Default is false
     is_preferred: bool,
     /// Default is None
+    #[debug("address family selector callback")]
     address_family_selector: Option<Box<dyn Fn() -> BoxFuture<bool> + Send + Sync + 'static>>,
     /// Default is false
     is_prober: bool,
@@ -222,16 +225,6 @@ pub struct ClientBuilder {
     proxy_url: Option<Url>,
 }
 
-impl std::fmt::Debug for ClientBuilder {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let address_family_selector_txt = match self.address_family_selector {
-            Some(_) => "Some(Box<dyn Fn() -> BoxFuture<'static, bool> + Send + Sync + 'static>)",
-            None => "None",
-        };
-        write!(f, "ClientBuilder {{ can_ack_pings: {}, is_preferred: {}, address_family_selector: {address_family_selector_txt} }}", self.can_ack_pings, self.is_preferred)
-    }
-}
-
 impl ClientBuilder {
     /// Create a new [`ClientBuilder`]
     pub fn new(url: impl Into<RelayUrl>) -> Self {
@@ -620,7 +613,7 @@ impl Actor {
 
     async fn connect_ws(&self) -> Result<(ConnReader, ConnWriter), ClientError> {
         let mut dial_url = (*self.url).clone();
-        dial_url.set_path("/derp");
+        dial_url.set_path(RELAY_HTTP_PATH);
 
         debug!(%dial_url, "Dialing relay by websocket");
 
@@ -706,7 +699,7 @@ impl Actor {
         );
         debug!("Sending upgrade request");
         let req = Request::builder()
-            .uri("/derp")
+            .uri(RELAY_HTTP_PATH)
             .header(UPGRADE, Protocol::Relay.upgrade_header())
             .body(http_body_util::Empty::<hyper::body::Bytes>::new())?;
         request_sender.send_request(req).await.map_err(From::from)

iroh-net/src/relay/http/server.rs

@@ -30,6 +30,8 @@ use crate::relay::http::{
 use crate::relay::server::{ClientConnHandler, MaybeTlsStream};
 use crate::relay::MaybeTlsStreamServer;
 
+use super::RELAY_HTTP_PATH;
+
 type BytesBody = http_body_util::Full<hyper::body::Bytes>;
 type HyperError = Box<dyn std::error::Error + Send + Sync>;
 type HyperResult<T> = std::result::Result<T, HyperError>;
@@ -226,8 +228,6 @@ pub struct ServerBuilder {
     /// Used when certain routes in your server should be made available at the same port as
     /// the relay server, and so must be handled along side requests to the relay endpoint.
     handlers: Handlers,
-    /// Defaults to `GET` request at "/derp".
-    relay_endpoint: &'static str,
     /// Use a custom relay response handler.
     ///
     /// Typically used when you want to disable any relay connections.
@@ -250,7 +250,6 @@ impl ServerBuilder {
             addr,
             tls_config: None,
             handlers: Default::default(),
-            relay_endpoint: "/derp",
             relay_override: None,
             headers: HeaderMap::new(),
             not_found_fn: None,
@@ -296,14 +295,6 @@ impl ServerBuilder {
         self
     }
 
-    /// Sets a custom endpoint for the relay handler.
-    ///
-    /// The default is `/derp`.
-    pub fn relay_endpoint(mut self, endpoint: &'static str) -> Self {
-        self.relay_endpoint = endpoint;
-        self
-    }
-
     /// Adds HTTP headers to responses.
     pub fn headers(mut self, headers: HeaderMap) -> Self {
         for (k, v) in headers.iter() {
@@ -347,13 +338,7 @@ impl ServerBuilder {
             }),
         };
 
-        let service = RelayService::new(
-            self.handlers,
-            relay_handler,
-            self.relay_endpoint,
-            not_found_fn,
-            self.headers,
-        );
+        let service = RelayService::new(self.handlers, relay_handler, not_found_fn, self.headers);
 
         let server_state = ServerState {
             addr: self.addr,
@@ -564,7 +549,11 @@ impl Service<Request<Incoming>> for RelayService {
 
     fn call(&self, req: Request<Incoming>) -> Self::Future {
         // if the request hits the relay endpoint
-        if req.method() == hyper::Method::GET && req.uri().path() == self.0.relay_endpoint {
+        // or /derp for backwards compat
+        if matches!(
+            (req.method(), req.uri().path()),
+            (&hyper::Method::GET, "/derp" | RELAY_HTTP_PATH)
+        ) {
             match &self.0.relay_handler {
                 RelayHandler::Override(f) => {
                     // see if we have some override response
@@ -597,7 +586,6 @@ struct RelayService(Arc<Inner>);
 #[derive(derive_more::Debug)]
 struct Inner {
     pub relay_handler: RelayHandler,
-    pub relay_endpoint: &'static str,
     #[debug("Box<Fn(ResponseBuilder) -> Result<Response<BytesBody>> + Send + Sync + 'static>")]
     pub not_found_fn: HyperHandler,
     pub handlers: Handlers,
@@ -645,14 +633,12 @@ impl RelayService {
     fn new(
         handlers: Handlers,
         relay_handler: RelayHandler,
-        relay_endpoint: &'static str,
         not_found_fn: HyperHandler,
         headers: HeaderMap,
     ) -> Self {
         Self(Arc::new(Inner {
             relay_handler,
             handlers,
-            relay_endpoint,
             not_found_fn,
             headers,
         }))

iroh-net/src/relay/iroh_relay.rs

@@ -27,7 +27,7 @@ use tracing::{debug, error, info, info_span, instrument, trace, warn, Instrument
 
 use crate::key::SecretKey;
 use crate::relay;
-use crate::relay::http::{ServerBuilder as RelayServerBuilder, TlsAcceptor};
+use crate::relay::http::{ServerBuilder as RelayServerBuilder, TlsAcceptor, RELAY_PROBE_HTTP_PATH};
 use crate::stun;
 use crate::util::AbortingJoinHandle;
 
@@ -76,7 +76,7 @@ pub struct ServerConfig<EC: fmt::Debug, EA: fmt::Debug = EC> {
 
 /// Configuration for the Relay HTTP and HTTPS server.
 ///
-/// This includes the HTTP services hosted by the Relay server, the Relay `/derp` HTTP
+/// This includes the HTTP services hosted by the Relay server, the Relay `/relay` HTTP
 /// endpoint is only one of the services served.
 #[derive(Debug)]
 pub struct RelayConfig<EC: fmt::Debug, EA: fmt::Debug = EC> {
@@ -235,7 +235,8 @@ impl Server {
                     .relay_override(Box::new(relay_disabled_handler))
                     .request_handler(Method::GET, "/", Box::new(root_handler))
                     .request_handler(Method::GET, "/index.html", Box::new(root_handler))
-                    .request_handler(Method::GET, "/derp/probe", Box::new(probe_handler))
+                    .request_handler(Method::GET, "/derp/probe", Box::new(probe_handler)) // backwards compat
+                    .request_handler(Method::GET, RELAY_PROBE_HTTP_PATH, Box::new(probe_handler))
                     .request_handler(Method::GET, "/robots.txt", Box::new(robots_handler));
                 let http_addr = match relay_config.tls {
                     Some(tls_config) => {
@@ -702,14 +703,29 @@ mod tests {
     use std::time::Duration;
 
     use bytes::Bytes;
+    use http::header::UPGRADE;
     use iroh_base::node_addr::RelayUrl;
 
-    use crate::relay::http::ClientBuilder;
+    use crate::relay::http::{ClientBuilder, HTTP_UPGRADE_PROTOCOL};
 
     use self::relay::ReceivedMessage;
 
     use super::*;
 
+    async fn spawn_local_relay() -> Result<Server> {
+        Server::spawn(ServerConfig::<(), ()> {
+            relay: Some(RelayConfig {
+                secret_key: SecretKey::generate(),
+                http_bind_addr: (Ipv4Addr::LOCALHOST, 0).into(),
+                tls: None,
+                limits: Default::default(),
+            }),
+            stun: None,
+            metrics_addr: None,
+        })
+        .await
+    }
+
     #[tokio::test]
     async fn test_no_services() {
         let _guard = iroh_test::logging::setup();
@@ -748,18 +764,7 @@ mod tests {
     #[tokio::test]
     async fn test_root_handler() {
         let _guard = iroh_test::logging::setup();
-        let server = Server::spawn(ServerConfig::<(), ()> {
-            relay: Some(RelayConfig {
-                secret_key: SecretKey::generate(),
-                http_bind_addr: (Ipv4Addr::LOCALHOST, 0).into(),
-                tls: None,
-                limits: Default::default(),
-            }),
-            stun: None,
-            metrics_addr: None,
-        })
-        .await
-        .unwrap();
+        let server = spawn_local_relay().await.unwrap();
         let url = format!("http://{}", server.http_addr().unwrap());
 
         let response = reqwest::get(&url).await.unwrap();
@@ -771,18 +776,7 @@ mod tests {
     #[tokio::test]
     async fn test_captive_portal_service() {
         let _guard = iroh_test::logging::setup();
-        let server = Server::spawn(ServerConfig::<(), ()> {
-            relay: Some(RelayConfig {
-                secret_key: SecretKey::generate(),
-                http_bind_addr: (Ipv4Addr::LOCALHOST, 0).into(),
-                tls: None,
-                limits: Default::default(),
-            }),
-            stun: None,
-            metrics_addr: None,
-        })
-        .await
-        .unwrap();
+        let server = spawn_local_relay().await.unwrap();
         let url = format!("http://{}/generate_204", server.http_addr().unwrap());
         let challenge = "123az__.";
 
@@ -800,21 +794,28 @@ mod tests {
         assert!(body.is_empty());
     }
 
+    #[tokio::test]
+    async fn test_relay_client_legacy_route() {
+        let _guard = iroh_test::logging::setup();
+        let server = spawn_local_relay().await.unwrap();
+        // We're testing the legacy endpoint at `/derp`
+        let endpoint_url = format!("http://{}/derp", server.http_addr().unwrap());
+
+        let client = reqwest::Client::new();
+        let result = client
+            .get(endpoint_url)
+            .header(UPGRADE, HTTP_UPGRADE_PROTOCOL)
+            .send()
+            .await
+            .unwrap();
+
+        assert_eq!(result.status(), StatusCode::SWITCHING_PROTOCOLS);
+    }
+
     #[tokio::test]
     async fn test_relay_clients_both_derp() {
         let _guard = iroh_test::logging::setup();
-        let server = Server::spawn(ServerConfig::<(), ()> {
-            relay: Some(RelayConfig {
-                secret_key: SecretKey::generate(),
-                http_bind_addr: (Ipv4Addr::LOCALHOST, 0).into(),
-                tls: None,
-                limits: Default::default(),
-            }),
-            stun: None,
-            metrics_addr: None,
-        })
-        .await
-        .unwrap();
+        let server = spawn_local_relay().await.unwrap();
         let relay_url = format!("http://{}", server.http_addr().unwrap());
         let relay_url: RelayUrl = relay_url.parse().unwrap();
 
@@ -879,18 +880,7 @@ mod tests {
     #[tokio::test]
     async fn test_relay_clients_both_websockets() {
         let _guard = iroh_test::logging::setup();
-        let server = Server::spawn(ServerConfig::<(), ()> {
-            relay: Some(RelayConfig {
-                secret_key: SecretKey::generate(),
-                http_bind_addr: (Ipv4Addr::LOCALHOST, 0).into(),
-                tls: None,
-                limits: Default::default(),
-            }),
-            stun: None,
-            metrics_addr: None,
-        })
-        .await
-        .unwrap();
+        let server = spawn_local_relay().await.unwrap();
         // NOTE: Using `ws://` URL scheme to trigger websockets.
         let relay_url = format!("ws://{}", server.http_addr().unwrap());
         let relay_url: RelayUrl = relay_url.parse().unwrap();
@@ -956,18 +946,7 @@ mod tests {
     #[tokio::test]
     async fn test_relay_clients_websocket_and_derp() {
         let _guard = iroh_test::logging::setup();
-        let server = Server::spawn(ServerConfig::<(), ()> {
-            relay: Some(RelayConfig {
-                secret_key: SecretKey::generate(),
-                http_bind_addr: (Ipv4Addr::LOCALHOST, 0).into(),
-                tls: None,
-                limits: Default::default(),
-            }),
-            stun: None,
-            metrics_addr: None,
-        })
-        .await
-        .unwrap();
+        let server = spawn_local_relay().await.unwrap();
 
         let derp_relay_url = format!("http://{}", server.http_addr().unwrap());
         let derp_relay_url: RelayUrl = derp_relay_url.parse().unwrap();