From 1318fdb8664b703c1955725e7f7eea5ae172e762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Wed, 26 Jun 2024 16:54:41 +0200 Subject: [PATCH 1/7] refactor(iroh-relay)!: Remove `relay_endpoint` config option & rename `/derp` route to `/relay` But still keep accepting connections over `/derp` for backwards compatibility. --- iroh-net/src/relay/http.rs | 4 +++ iroh-net/src/relay/http/client.rs | 5 ++-- iroh-net/src/relay/http/server.rs | 50 +++++++++++-------------------- iroh-net/src/relay/iroh_relay.rs | 5 ++-- 4 files changed, 28 insertions(+), 36 deletions(-) diff --git a/iroh-net/src/relay/http.rs b/iroh-net/src/relay/http.rs index a61d17b798..e6ecb944a6 100644 --- a/iroh-net/src/relay/http.rs +++ b/iroh-net/src/relay/http.rs @@ -12,6 +12,10 @@ 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"; + #[cfg(test)] mod tests { use super::*; diff --git a/iroh-net/src/relay/http/client.rs b/iroh-net/src/relay/http/client.rs index f4d8957832..eb16c0f835 100644 --- a/iroh-net/src/relay/http/client.rs +++ b/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, @@ -620,7 +621,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 +707,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::::new())?; request_sender.send_request(req).await.map_err(From::from) diff --git a/iroh-net/src/relay/http/server.rs b/iroh-net/src/relay/http/server.rs index f21cb75460..9261916d6f 100644 --- a/iroh-net/src/relay/http/server.rs +++ b/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; type HyperError = Box; type HyperResult = std::result::Result; @@ -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,19 +549,23 @@ impl Service> for RelayService { fn call(&self, req: Request) -> Self::Future { // if the request hits the relay endpoint - if req.method() == hyper::Method::GET && req.uri().path() == self.0.relay_endpoint { - match &self.0.relay_handler { - RelayHandler::Override(f) => { - // see if we have some override response - let res = f(req, self.0.default_response()); - return Box::pin(async move { res }); - } - RelayHandler::ConnHandler(handler) => { - let h = handler.clone(); - // otherwise handle the relay connection as normal - return Box::pin(async move { h.call(req).await.map_err(Into::into) }); + match (req.method(), req.uri().path()) { + // /derp for backwards compat + (&hyper::Method::GET, "/derp" | RELAY_HTTP_PATH) => { + match &self.0.relay_handler { + RelayHandler::Override(f) => { + // see if we have some override response + let res = f(req, self.0.default_response()); + return Box::pin(async move { res }); + } + RelayHandler::ConnHandler(handler) => { + let h = handler.clone(); + // otherwise handle the relay connection as normal + return Box::pin(async move { h.call(req).await.map_err(Into::into) }); + } } } + _ => {} } // check all other possible endpoints let uri = req.uri().clone(); @@ -597,7 +586,6 @@ struct RelayService(Arc); #[derive(derive_more::Debug)] struct Inner { pub relay_handler: RelayHandler, - pub relay_endpoint: &'static str, #[debug("Box Result> + 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, })) diff --git a/iroh-net/src/relay/iroh_relay.rs b/iroh-net/src/relay/iroh_relay.rs index 253b7077f0..d66545d7c4 100644 --- a/iroh-net/src/relay/iroh_relay.rs +++ b/iroh-net/src/relay/iroh_relay.rs @@ -76,7 +76,7 @@ pub struct ServerConfig { /// 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 { @@ -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", Box::new(probe_handler)) .request_handler(Method::GET, "/robots.txt", Box::new(robots_handler)); let http_addr = match relay_config.tls { Some(tls_config) => { From 9acb4d40495ddc8d109f7c4f930d7a116c22e48b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Wed, 26 Jun 2024 16:57:49 +0200 Subject: [PATCH 2/7] refactor: Rewrite `match` into `if matches!` --- iroh-net/src/relay/http/server.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/iroh-net/src/relay/http/server.rs b/iroh-net/src/relay/http/server.rs index 9261916d6f..a6b4c08f05 100644 --- a/iroh-net/src/relay/http/server.rs +++ b/iroh-net/src/relay/http/server.rs @@ -549,23 +549,23 @@ impl Service> for RelayService { fn call(&self, req: Request) -> Self::Future { // if the request hits the relay endpoint - match (req.method(), req.uri().path()) { - // /derp for backwards compat - (&hyper::Method::GET, "/derp" | RELAY_HTTP_PATH) => { - match &self.0.relay_handler { - RelayHandler::Override(f) => { - // see if we have some override response - let res = f(req, self.0.default_response()); - return Box::pin(async move { res }); - } - RelayHandler::ConnHandler(handler) => { - let h = handler.clone(); - // otherwise handle the relay connection as normal - return Box::pin(async move { h.call(req).await.map_err(Into::into) }); - } + // 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 + let res = f(req, self.0.default_response()); + return Box::pin(async move { res }); + } + RelayHandler::ConnHandler(handler) => { + let h = handler.clone(); + // otherwise handle the relay connection as normal + return Box::pin(async move { h.call(req).await.map_err(Into::into) }); } } - _ => {} } // check all other possible endpoints let uri = req.uri().clone(); From 7e8c324c20ddbc1d4ac756a450a089dc8930b2df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Wed, 26 Jun 2024 17:00:38 +0200 Subject: [PATCH 3/7] refactor: Also make `"/relay/probe"` a const --- iroh-net/src/relay/http.rs | 2 ++ iroh-net/src/relay/iroh_relay.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/iroh-net/src/relay/http.rs b/iroh-net/src/relay/http.rs index e6ecb944a6..ed16884b94 100644 --- a/iroh-net/src/relay/http.rs +++ b/iroh-net/src/relay/http.rs @@ -15,6 +15,8 @@ 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 { diff --git a/iroh-net/src/relay/iroh_relay.rs b/iroh-net/src/relay/iroh_relay.rs index d66545d7c4..b58d7b9387 100644 --- a/iroh-net/src/relay/iroh_relay.rs +++ b/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; @@ -236,7 +236,7 @@ impl Server { .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)) // backwards compat - .request_handler(Method::GET, "/relay/probe", Box::new(probe_handler)) + .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) => { From 770231d798243afc078d6160d322169946c33873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Thu, 27 Jun 2024 16:49:36 +0200 Subject: [PATCH 4/7] chore: Test that the legacy `/derp` route keeps working --- iroh-net/src/relay/http/client.rs | 28 +++++---- iroh-net/src/relay/iroh_relay.rs | 96 ++++++++++++------------------- 2 files changed, 53 insertions(+), 71 deletions(-) diff --git a/iroh-net/src/relay/http/client.rs b/iroh-net/src/relay/http/client.rs index eb16c0f835..b2c544fed3 100644 --- a/iroh-net/src/relay/http/client.rs +++ b/iroh-net/src/relay/http/client.rs @@ -178,6 +178,7 @@ struct Actor { ping_tasks: JoinSet<()>, dns_resolver: DnsResolver, proxy_url: Option, + relay_path: &'static str, } #[derive(Default, Debug)] @@ -203,12 +204,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 BoxFuture + Send + Sync + 'static>>, /// Default is false is_prober: bool, @@ -221,16 +224,8 @@ pub struct ClientBuilder { insecure_skip_cert_verify: bool, /// HTTP Proxy proxy_url: Option, -} - -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 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) - } + /// Default is "/relay" + relay_path: &'static str, } impl ClientBuilder { @@ -246,6 +241,7 @@ impl ClientBuilder { #[cfg(any(test, feature = "test-utils"))] insecure_skip_cert_verify: false, proxy_url: None, + relay_path: RELAY_HTTP_PATH, } } @@ -303,6 +299,15 @@ impl ClientBuilder { self } + /// Set the relay endpoint path used for making connections. + /// + /// Private for now, as it's only used in tests so far. + #[cfg(test)] + pub(crate) fn relay_path(mut self, relay_path: &'static str) -> Self { + self.relay_path = relay_path; + self + } + /// Build the [`Client`] pub fn build(self, key: SecretKey, dns_resolver: DnsResolver) -> (Client, ClientReceiver) { // TODO: review TLS config @@ -345,6 +350,7 @@ impl ClientBuilder { tls_connector, dns_resolver, proxy_url: self.proxy_url, + relay_path: self.relay_path, }; let (msg_sender, inbox) = mpsc::channel(64); @@ -621,7 +627,7 @@ impl Actor { async fn connect_ws(&self) -> Result<(ConnReader, ConnWriter), ClientError> { let mut dial_url = (*self.url).clone(); - dial_url.set_path(RELAY_HTTP_PATH); + dial_url.set_path(self.relay_path); debug!(%dial_url, "Dialing relay by websocket"); diff --git a/iroh-net/src/relay/iroh_relay.rs b/iroh-net/src/relay/iroh_relay.rs index b58d7b9387..6cd4f698cd 100644 --- a/iroh-net/src/relay/iroh_relay.rs +++ b/iroh-net/src/relay/iroh_relay.rs @@ -711,6 +711,20 @@ mod tests { use super::*; + async fn spawn_local_relay() -> Result { + 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(); @@ -749,18 +763,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(); @@ -772,18 +775,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__."; @@ -801,21 +793,27 @@ 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(); + let relay_url = format!("http://{}", server.http_addr().unwrap()); + let relay_url: RelayUrl = relay_url.parse().unwrap(); + + // set up client a + let secret_key = SecretKey::generate(); + let resolver = crate::dns::default_resolver().clone(); + let (client, _) = ClientBuilder::new(relay_url) + .relay_path("/derp") // Try the legacy relay path for backwards compatibility + .build(secret_key, resolver); + + client.ping().await.unwrap(); + } + #[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(); @@ -880,18 +878,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(); @@ -957,18 +944,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(); From 69fb5ef7ff08ffaf46ddc0dfbec5378554c00c4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Mon, 1 Jul 2024 13:42:34 +0200 Subject: [PATCH 5/7] refactor: Remove `relay_path` and simplify test --- iroh-net/src/relay/http/client.rs | 16 +--------------- iroh-net/src/relay/iroh_relay.rs | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/iroh-net/src/relay/http/client.rs b/iroh-net/src/relay/http/client.rs index b2c544fed3..045213012d 100644 --- a/iroh-net/src/relay/http/client.rs +++ b/iroh-net/src/relay/http/client.rs @@ -178,7 +178,6 @@ struct Actor { ping_tasks: JoinSet<()>, dns_resolver: DnsResolver, proxy_url: Option, - relay_path: &'static str, } #[derive(Default, Debug)] @@ -224,8 +223,6 @@ pub struct ClientBuilder { insecure_skip_cert_verify: bool, /// HTTP Proxy proxy_url: Option, - /// Default is "/relay" - relay_path: &'static str, } impl ClientBuilder { @@ -241,7 +238,6 @@ impl ClientBuilder { #[cfg(any(test, feature = "test-utils"))] insecure_skip_cert_verify: false, proxy_url: None, - relay_path: RELAY_HTTP_PATH, } } @@ -299,15 +295,6 @@ impl ClientBuilder { self } - /// Set the relay endpoint path used for making connections. - /// - /// Private for now, as it's only used in tests so far. - #[cfg(test)] - pub(crate) fn relay_path(mut self, relay_path: &'static str) -> Self { - self.relay_path = relay_path; - self - } - /// Build the [`Client`] pub fn build(self, key: SecretKey, dns_resolver: DnsResolver) -> (Client, ClientReceiver) { // TODO: review TLS config @@ -350,7 +337,6 @@ impl ClientBuilder { tls_connector, dns_resolver, proxy_url: self.proxy_url, - relay_path: self.relay_path, }; let (msg_sender, inbox) = mpsc::channel(64); @@ -627,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(self.relay_path); + dial_url.set_path(RELAY_HTTP_PATH); debug!(%dial_url, "Dialing relay by websocket"); diff --git a/iroh-net/src/relay/iroh_relay.rs b/iroh-net/src/relay/iroh_relay.rs index 6cd4f698cd..b16095bb6a 100644 --- a/iroh-net/src/relay/iroh_relay.rs +++ b/iroh-net/src/relay/iroh_relay.rs @@ -703,9 +703,10 @@ 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; @@ -797,17 +798,18 @@ mod tests { async fn test_relay_client_legacy_route() { let _guard = iroh_test::logging::setup(); let server = spawn_local_relay().await.unwrap(); - let relay_url = format!("http://{}", server.http_addr().unwrap()); - let relay_url: RelayUrl = relay_url.parse().unwrap(); + // We're testing the legacy endpoint at `/derp` + let endpoint_url = format!("http://{}/derp", server.http_addr().unwrap()); - // set up client a - let secret_key = SecretKey::generate(); - let resolver = crate::dns::default_resolver().clone(); - let (client, _) = ClientBuilder::new(relay_url) - .relay_path("/derp") // Try the legacy relay path for backwards compatibility - .build(secret_key, resolver); + let client = reqwest::Client::new(); + let result = client + .get(endpoint_url) + .header(UPGRADE, HTTP_UPGRADE_PROTOCOL) + .send() + .await + .unwrap(); - client.ping().await.unwrap(); + assert_eq!(result.status(), StatusCode::SWITCHING_PROTOCOLS); } #[tokio::test] From ceab5038781c49e71620a05396fb5d4002f1ae97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Tue, 2 Jul 2024 08:33:54 +0200 Subject: [PATCH 6/7] refactor: Introduce more constants for paths --- iroh-net/src/relay/http.rs | 14 ++++++++++---- iroh-net/src/relay/http/client.rs | 6 +++--- iroh-net/src/relay/http/server.rs | 4 ++-- iroh-net/src/relay/iroh_relay.rs | 12 +++++++++--- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/iroh-net/src/relay/http.rs b/iroh-net/src/relay/http.rs index ed16884b94..dd5b180871 100644 --- a/iroh-net/src/relay/http.rs +++ b/iroh-net/src/relay/http.rs @@ -12,11 +12,17 @@ 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 +/// The HTTP 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"; +pub const RELAY_PATH: &str = "/relay"; +/// The HTTP path under which the relay allows doing latency queries for testing. +pub const RELAY_PROBE_PATH: &str = "/relay/probe"; +/// The legacy HTTP path under which the relay used to accept relaying connections. +/// We keep this for backwards compatibility. +pub(crate) const LEGACY_RELAY_PATH: &str = "/derp"; +/// The legacy HTTP path under which the relay used to allow latency queries. +/// We keep this for backwards compatibility. +pub(crate) const LEGACY_RELAY_PROBE_PATH: &str = "/derp/probe"; #[cfg(test)] mod tests { diff --git a/iroh-net/src/relay/http/client.rs b/iroh-net/src/relay/http/client.rs index 045213012d..ec05149928 100644 --- a/iroh-net/src/relay/http/client.rs +++ b/iroh-net/src/relay/http/client.rs @@ -31,7 +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::http::RELAY_PATH; use crate::relay::RelayUrl; use crate::relay::{ client::Client as RelayClient, client::ClientBuilder as RelayClientBuilder, @@ -613,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(RELAY_HTTP_PATH); + dial_url.set_path(RELAY_PATH); debug!(%dial_url, "Dialing relay by websocket"); @@ -699,7 +699,7 @@ impl Actor { ); debug!("Sending upgrade request"); let req = Request::builder() - .uri(RELAY_HTTP_PATH) + .uri(RELAY_PATH) .header(UPGRADE, Protocol::Relay.upgrade_header()) .body(http_body_util::Empty::::new())?; request_sender.send_request(req).await.map_err(From::from) diff --git a/iroh-net/src/relay/http/server.rs b/iroh-net/src/relay/http/server.rs index a6b4c08f05..7057117283 100644 --- a/iroh-net/src/relay/http/server.rs +++ b/iroh-net/src/relay/http/server.rs @@ -30,7 +30,7 @@ use crate::relay::http::{ use crate::relay::server::{ClientConnHandler, MaybeTlsStream}; use crate::relay::MaybeTlsStreamServer; -use super::RELAY_HTTP_PATH; +use super::{LEGACY_RELAY_PATH, RELAY_PATH}; type BytesBody = http_body_util::Full; type HyperError = Box; @@ -552,7 +552,7 @@ impl Service> for RelayService { // or /derp for backwards compat if matches!( (req.method(), req.uri().path()), - (&hyper::Method::GET, "/derp" | RELAY_HTTP_PATH) + (&hyper::Method::GET, LEGACY_RELAY_PATH | RELAY_PATH) ) { match &self.0.relay_handler { RelayHandler::Override(f) => { diff --git a/iroh-net/src/relay/iroh_relay.rs b/iroh-net/src/relay/iroh_relay.rs index b16095bb6a..d71c91fc39 100644 --- a/iroh-net/src/relay/iroh_relay.rs +++ b/iroh-net/src/relay/iroh_relay.rs @@ -27,7 +27,9 @@ 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, RELAY_PROBE_HTTP_PATH}; +use crate::relay::http::{ + ServerBuilder as RelayServerBuilder, TlsAcceptor, LEGACY_RELAY_PROBE_PATH, RELAY_PROBE_PATH, +}; use crate::stun; use crate::util::AbortingJoinHandle; @@ -235,8 +237,12 @@ 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)) // backwards compat - .request_handler(Method::GET, RELAY_PROBE_HTTP_PATH, Box::new(probe_handler)) + .request_handler( + Method::GET, + LEGACY_RELAY_PROBE_PATH, + Box::new(probe_handler), + ) // backwards compat + .request_handler(Method::GET, RELAY_PROBE_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) => { From a142c57c271b9b40030d152cb5dbced4f758657e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Tue, 2 Jul 2024 08:42:48 +0200 Subject: [PATCH 7/7] refactor: Split out the client changes from this PR --- iroh-net/src/relay/http/client.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/iroh-net/src/relay/http/client.rs b/iroh-net/src/relay/http/client.rs index ec05149928..f4d8957832 100644 --- a/iroh-net/src/relay/http/client.rs +++ b/iroh-net/src/relay/http/client.rs @@ -31,7 +31,6 @@ 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_PATH; use crate::relay::RelayUrl; use crate::relay::{ client::Client as RelayClient, client::ClientBuilder as RelayClientBuilder, @@ -203,14 +202,12 @@ 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 BoxFuture + Send + Sync + 'static>>, /// Default is false is_prober: bool, @@ -225,6 +222,16 @@ pub struct ClientBuilder { proxy_url: Option, } +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 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) -> Self { @@ -613,7 +620,7 @@ impl Actor { async fn connect_ws(&self) -> Result<(ConnReader, ConnWriter), ClientError> { let mut dial_url = (*self.url).clone(); - dial_url.set_path(RELAY_PATH); + dial_url.set_path("/derp"); debug!(%dial_url, "Dialing relay by websocket"); @@ -699,7 +706,7 @@ impl Actor { ); debug!("Sending upgrade request"); let req = Request::builder() - .uri(RELAY_PATH) + .uri("/derp") .header(UPGRADE, Protocol::Relay.upgrade_header()) .body(http_body_util::Empty::::new())?; request_sender.send_request(req).await.map_err(From::from)