From eb1127ab52f1d7a62112140af5902af858bb0b8e Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Mon, 7 Aug 2023 22:47:14 +0200 Subject: [PATCH 1/4] refactor(client): support default port number --- client/http-client/Cargo.toml | 1 + client/http-client/src/transport.rs | 74 ++++++-------- client/transport/Cargo.toml | 2 + client/transport/src/ws/mod.rs | 143 ++++++++++++++-------------- client/ws-client/Cargo.toml | 1 + client/ws-client/src/lib.rs | 5 +- examples/examples/core_client.rs | 4 +- tests/tests/integration_tests.rs | 20 +--- 8 files changed, 113 insertions(+), 137 deletions(-) diff --git a/client/http-client/Cargo.toml b/client/http-client/Cargo.toml index e0ba075706..70f45c3713 100644 --- a/client/http-client/Cargo.toml +++ b/client/http-client/Cargo.toml @@ -25,6 +25,7 @@ thiserror = "1.0" tokio = { version = "1.16", features = ["time"] } tracing = "0.1.34" tower = { version = "0.4.13", features = ["util"] } +url = "2.4.0" [dev-dependencies] tracing-subscriber = { version = "0.3.3", features = ["env-filter"] } diff --git a/client/http-client/src/transport.rs b/client/http-client/src/transport.rs index 743c8bdda5..2b618299cc 100644 --- a/client/http-client/src/transport.rs +++ b/client/http-client/src/transport.rs @@ -9,7 +9,6 @@ use hyper::body::{Body, HttpBody}; use hyper::client::{Client, HttpConnector}; use hyper::http::{HeaderMap, HeaderValue}; -use hyper::Uri; use jsonrpsee_core::client::CertificateStore; use jsonrpsee_core::error::GenericTransportError; use jsonrpsee_core::http_helpers; @@ -20,6 +19,7 @@ use std::pin::Pin; use std::task::{Context, Poll}; use thiserror::Error; use tower::{Layer, Service, ServiceExt}; +use url::Url; const CONTENT_TYPE_JSON: &str = "application/json"; @@ -77,7 +77,7 @@ where #[derive(Debug, Clone)] pub struct HttpTransportClient { /// Target to connect to. - target: ParsedUri, + target: String, /// HTTP client client: S, /// Configurable max request body size @@ -109,13 +109,17 @@ where headers: HeaderMap, service_builder: tower::ServiceBuilder, ) -> Result { - let uri = ParsedUri::try_from(target.as_ref())?; + let mut url = Url::parse(target.as_ref()).map_err(|e| Error::Url(format!("Invalid URL: {e}")))?; + if url.host_str().is_none() { + return Err(Error::Url("Invalid host".into())); + } + url.set_fragment(None); - let client = match uri.0.scheme_str() { + let client = match url.scheme() { #[cfg(not(feature = "__tls"))] - Some("http") => HttpBackend::Http(Client::new()), + "http" => HttpBackend::Http(Client::new()), #[cfg(feature = "__tls")] - Some("https") | Some("http") => { + "https" | "http" => { let connector = match cert_store { #[cfg(feature = "native-tls")] CertificateStore::Native => hyper_rustls::HttpsConnectorBuilder::new() @@ -155,7 +159,7 @@ where } Ok(Self { - target: uri, + target: url.as_str().to_owned(), client: service_builder.service(client), max_request_size, max_response_size, @@ -171,7 +175,7 @@ where return Err(Error::RequestTooLarge); } - let mut req = hyper::Request::post(&self.target.0); + let mut req = hyper::Request::post(&self.target); if let Some(headers) = req.headers_mut() { *headers = self.headers.clone(); } @@ -204,22 +208,6 @@ where } } -#[derive(Debug, Clone)] -struct ParsedUri(Uri); - -impl TryFrom<&str> for ParsedUri { - type Error = Error; - - fn try_from(target: &str) -> Result { - let uri: Uri = target.parse().map_err(|e| Error::Url(format!("Invalid URL: {e}")))?; - if uri.port_u16().is_none() { - Err(Error::Url("Port number is missing in the URL".into())) - } else { - Ok(ParsedUri(uri)) - } - } -} - /// Error that can happen during a request. #[derive(Debug, Error)] pub enum Error { @@ -272,21 +260,6 @@ mod tests { use super::*; use jsonrpsee_core::client::CertificateStore; - fn assert_target( - client: &HttpTransportClient, - host: &str, - scheme: &str, - path_and_query: &str, - port: u16, - max_request_size: u32, - ) { - assert_eq!(client.target.0.scheme_str(), Some(scheme)); - assert_eq!(client.target.0.path_and_query().map(|pq| pq.as_str()), Some(path_and_query)); - assert_eq!(client.target.0.host(), Some(host)); - assert_eq!(client.target.0.port_u16(), Some(port)); - assert_eq!(client.max_request_size, max_request_size); - } - #[test] fn invalid_http_url_rejected() { let err = HttpTransportClient::new( @@ -315,7 +288,7 @@ mod tests { tower::ServiceBuilder::new(), ) .unwrap(); - assert_target(&client, "localhost", "https", "/", 9933, 80); + assert_eq!(&client.target, "https://localhost:9933/"); } #[cfg(not(feature = "__tls"))] @@ -372,7 +345,7 @@ mod tests { tower::ServiceBuilder::new(), ) .unwrap(); - assert_target(&client, "localhost", "http", "/my-special-path", 9944, 1337); + assert_eq!(&client.target, "http://localhost:9944/my-special-path"); } #[test] @@ -387,7 +360,7 @@ mod tests { tower::ServiceBuilder::new(), ) .unwrap(); - assert_target(&client, "127.0.0.1", "http", "/my?name1=value1&name2=value2", 9999, u32::MAX); + assert_eq!(&client.target, "http://127.0.0.1:9999/my?name1=value1&name2=value2"); } #[test] @@ -402,7 +375,22 @@ mod tests { tower::ServiceBuilder::new(), ) .unwrap(); - assert_target(&client, "127.0.0.1", "http", "/my.htm", 9944, 999); + assert_eq!(&client.target, "http://127.0.0.1:9944/my.htm"); + } + + #[test] + fn url_default_port_works() { + let client = HttpTransportClient::new( + 999, + "http://127.0.0.1", + 999, + CertificateStore::Native, + 80, + HeaderMap::new(), + tower::ServiceBuilder::new(), + ) + .unwrap(); + assert_eq!(&client.target, "http://127.0.0.1/"); } #[tokio::test] diff --git a/client/transport/Cargo.toml b/client/transport/Cargo.toml index 822152ae2c..40363f052c 100644 --- a/client/transport/Cargo.toml +++ b/client/transport/Cargo.toml @@ -25,6 +25,7 @@ http = { version = "0.2", optional = true } tokio-util = { version = "0.7", features = ["compat"], optional = true } tokio = { version = "1.16", features = ["net", "time", "macros"], optional = true } pin-project = { version = "1", optional = true } +url = { version = "2.4.0", optional = true } # tls rustls-native-certs = { version = "0.6", optional = true } @@ -53,6 +54,7 @@ ws = [ "soketto", "pin-project", "thiserror", + "url", ] web = [ "gloo-net", diff --git a/client/transport/src/ws/mod.rs b/client/transport/src/ws/mod.rs index 9bc3f322ed..a26d81a934 100644 --- a/client/transport/src/ws/mod.rs +++ b/client/transport/src/ws/mod.rs @@ -27,7 +27,7 @@ mod stream; use std::io; -use std::net::{SocketAddr, ToSocketAddrs}; +use std::net::SocketAddr; use std::time::Duration; use futures_util::io::{BufReader, BufWriter}; @@ -44,6 +44,7 @@ use tokio::net::TcpStream; pub use http::{uri::InvalidUri, HeaderMap, HeaderValue, Uri}; pub use soketto::handshake::client::Header; +pub use url::Url; /// Sending end of WebSocket transport. #[derive(Debug)] @@ -275,7 +276,7 @@ impl TransportReceiverT for Receiver { impl WsTransportClientBuilder { /// Try to establish the connection. - pub async fn build(self, uri: Uri) -> Result<(Sender, Receiver), WsHandshakeError> { + pub async fn build(self, uri: Url) -> Result<(Sender, Receiver), WsHandshakeError> { let target: Target = uri.try_into()?; self.try_connect(target).await } @@ -348,54 +349,49 @@ impl WsTransportClientBuilder { } Ok(ServerResponse::Redirect { status_code, location }) => { tracing::debug!("Redirection: status_code: {}, location: {}", status_code, location); - match location.parse::() { + match url::Url::parse(&location) { // redirection with absolute path => need to lookup. Ok(uri) => { // Absolute URI. - if uri.scheme().is_some() { - target = uri.try_into().map_err(|e| { - tracing::error!("Redirection failed: {:?}", e); - e - })?; - - // Only build TLS connector if `wss` in redirection URL. - #[cfg(feature = "__tls")] - match target._mode { - Mode::Tls if connector.is_none() => { - connector = Some(build_tls_config(&self.certificate_store)?); - } - Mode::Tls => (), - // Drop connector if it was configured previously. - Mode::Plain => { - connector = None; + target = uri.try_into().map_err(|e| { + tracing::error!("Redirection failed: {:?}", e); + e + })?; + + // Only build TLS connector if `wss` in redirection URL. + #[cfg(feature = "__tls")] + match target._mode { + Mode::Tls if connector.is_none() => { + connector = Some(build_tls_config(&self.certificate_store)?); + } + Mode::Tls => (), + // Drop connector if it was configured previously. + Mode::Plain => { + connector = None; + } + }; + } + + // Relative URI. + Err(url::ParseError::RelativeUrlWithoutBase) + | Err(url::ParseError::RelativeUrlWithCannotBeABaseBase) => { + // Replace the entire path_and_query if `location` starts with `/` or `//`. + if location.starts_with('/') { + target.path_and_query = location; + } else { + match target.path_and_query.rfind('/') { + Some(offset) => target.path_and_query.replace_range(offset + 1.., &location), + None => { + let e = format!("path_and_query: {location}; this is a bug it must contain `/` please open issue"); + err = Some(Err(WsHandshakeError::Url(e.into()))); + continue; } }; } - // Relative URI. - else { - // Replace the entire path_and_query if `location` starts with `/` or `//`. - if location.starts_with('/') { - target.path_and_query = location; - } else { - match target.path_and_query.rfind('/') { - Some(offset) => { - target.path_and_query.replace_range(offset + 1.., &location) - } - None => { - err = Some(Err(WsHandshakeError::Url( - format!( - "path_and_query: {location}; this is a bug it must contain `/` please open issue" - ) - .into(), - ))); - continue; - } - }; - } - target.sockaddrs = sockaddrs; - } + target.sockaddrs = sockaddrs; break; } + Err(e) => { err = Some(Err(WsHandshakeError::Url(e.to_string().into()))); } @@ -488,38 +484,36 @@ pub struct Target { path_and_query: String, } -impl TryFrom for Target { +impl TryFrom for Target { type Error = WsHandshakeError; - fn try_from(uri: Uri) -> Result { - let _mode = match uri.scheme_str() { - Some("ws") => Mode::Plain, + fn try_from(url: Url) -> Result { + let _mode = match url.scheme() { + "ws" => Mode::Plain, #[cfg(feature = "__tls")] - Some("wss") => Mode::Tls, + "wss" => Mode::Tls, invalid_scheme => { - let scheme = invalid_scheme.unwrap_or("no scheme"); #[cfg(feature = "__tls")] - let err = format!("`{scheme}` not supported, expects 'ws' or 'wss'"); + let err = format!("`{invalid_scheme}` not supported, expects 'ws' or 'wss'"); #[cfg(not(feature = "__tls"))] let err = format!("`{}` not supported, expects 'ws' ('wss' requires the tls feature)", scheme); return Err(WsHandshakeError::Url(err.into())); } }; - let host = uri.host().map(ToOwned::to_owned).ok_or_else(|| WsHandshakeError::Url("No host in URL".into()))?; - let port = uri - .port_u16() - .ok_or_else(|| WsHandshakeError::Url("No port number in URL (default port is not supported)".into()))?; - let host_header = format!("{host}:{port}"); - let parts = uri.into_parts(); - let path_and_query = parts.path_and_query.ok_or_else(|| WsHandshakeError::Url("No path in URL".into()))?; - let sockaddrs = host_header.to_socket_addrs().map_err(WsHandshakeError::ResolutionFailed)?; - Ok(Self { - sockaddrs: sockaddrs.collect(), - host, - host_header, - _mode, - path_and_query: path_and_query.to_string(), - }) + let host = + url.host_str().map(ToOwned::to_owned).ok_or_else(|| WsHandshakeError::Url("No host in URL".into()))?; + let port = url.port_or_known_default().ok_or_else(|| WsHandshakeError::Url("Invalid port in URL".into()))?; + + let host_header = if port == 80 || port == 443 { host.clone() } else { format!("{host}:{port}") }; + + let mut path_and_query = url.path().to_owned(); + if let Some(query) = url.query() { + path_and_query.push('?'); + path_and_query.push_str(query); + } + + let sockaddrs = url.socket_addrs(|| None).map_err(WsHandshakeError::ResolutionFailed)?; + Ok(Self { sockaddrs, host, host_header, _mode, path_and_query: path_and_query.to_string() }) } } @@ -567,8 +561,7 @@ fn build_tls_config(cert_store: &CertificateStore) -> Result Result { - uri.parse::().map_err(|e: InvalidUri| WsHandshakeError::Url(e.to_string().into()))?.try_into() + Url::parse(uri).map_err(|e| WsHandshakeError::Url(e.to_string().into()))?.try_into() } #[test] @@ -587,11 +580,17 @@ mod tests { assert_ws_target(target, "127.0.0.1", "127.0.0.1:9933", Mode::Plain, "/"); } + #[test] + fn ws_default_port_works() { + let target = parse_target("ws://127.0.0.1").unwrap(); + assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/"); + } + #[cfg(feature = "__tls")] #[test] fn wss_works() { - let target = parse_target("wss://kusama-rpc.polkadot.io:443").unwrap(); - assert_ws_target(target, "kusama-rpc.polkadot.io", "kusama-rpc.polkadot.io:443", Mode::Tls, "/"); + let target = parse_target("wss://kusama-rpc.polkadot.io").unwrap(); + assert_ws_target(target, "kusama-rpc.polkadot.io", "kusama-rpc.polkadot.io", Mode::Tls, "/"); } #[cfg(not(feature = "__tls"))] @@ -618,18 +617,18 @@ mod tests { #[test] fn url_with_path_works() { let target = parse_target("ws://127.0.0.1:443/my-special-path").unwrap(); - assert_ws_target(target, "127.0.0.1", "127.0.0.1:443", Mode::Plain, "/my-special-path"); + assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/my-special-path"); } #[test] fn url_with_query_works() { let target = parse_target("ws://127.0.0.1:443/my?name1=value1&name2=value2").unwrap(); - assert_ws_target(target, "127.0.0.1", "127.0.0.1:443", Mode::Plain, "/my?name1=value1&name2=value2"); + assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/my?name1=value1&name2=value2"); } #[test] fn url_with_fragment_is_ignored() { let target = parse_target("ws://127.0.0.1:443/my.htm#ignore").unwrap(); - assert_ws_target(target, "127.0.0.1", "127.0.0.1:443", Mode::Plain, "/my.htm"); + assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/my.htm"); } } diff --git a/client/ws-client/Cargo.toml b/client/ws-client/Cargo.toml index 761ccf02d8..f4efd862ef 100644 --- a/client/ws-client/Cargo.toml +++ b/client/ws-client/Cargo.toml @@ -18,6 +18,7 @@ jsonrpsee-types = { workspace = true } jsonrpsee-client-transport = { workspace = true, features = ["ws"] } jsonrpsee-core = { workspace = true, features = ["async-client"] } http = "0.2.0" +url = "2.4.0" [dev-dependencies] tracing-subscriber = { version = "0.3.3", features = ["env-filter"] } diff --git a/client/ws-client/src/lib.rs b/client/ws-client/src/lib.rs index cce7048a3b..5b89b7353d 100644 --- a/client/ws-client/src/lib.rs +++ b/client/ws-client/src/lib.rs @@ -43,8 +43,9 @@ pub use jsonrpsee_types as types; pub use http::{HeaderMap, HeaderValue}; use std::time::Duration; +use url::Url; -use jsonrpsee_client_transport::ws::{InvalidUri, Uri, WsTransportClientBuilder}; +use jsonrpsee_client_transport::ws::WsTransportClientBuilder; use jsonrpsee_core::client::{CertificateStore, ClientBuilder, IdKind}; use jsonrpsee_core::{Error, TEN_MB_SIZE_BYTES}; @@ -243,7 +244,7 @@ impl WsClientBuilder { max_redirections, }; - let uri: Uri = url.as_ref().parse().map_err(|e: InvalidUri| Error::Transport(e.into()))?; + let uri = Url::parse(url.as_ref()).map_err(|e| Error::Transport(e.into()))?; let (sender, receiver) = transport_builder.build(uri).await.map_err(|e| Error::Transport(e.into()))?; let mut client = ClientBuilder::default() diff --git a/examples/examples/core_client.rs b/examples/examples/core_client.rs index 40b30ec43f..36e0926016 100644 --- a/examples/examples/core_client.rs +++ b/examples/examples/core_client.rs @@ -26,7 +26,7 @@ use std::net::SocketAddr; -use jsonrpsee::client_transport::ws::{Uri, WsTransportClientBuilder}; +use jsonrpsee::client_transport::ws::{Url, WsTransportClientBuilder}; use jsonrpsee::core::client::{Client, ClientBuilder, ClientT}; use jsonrpsee::rpc_params; use jsonrpsee::server::{RpcModule, Server}; @@ -39,7 +39,7 @@ async fn main() -> anyhow::Result<()> { .expect("setting default subscriber failed"); let addr = run_server().await?; - let uri: Uri = format!("ws://{}", addr).parse()?; + let uri = Url::parse(&format!("ws://{}", addr))?; let (tx, rx) = WsTransportClientBuilder::default().build(uri).await?; let client: Client = ClientBuilder::default().build_with_tokio(tx, rx); diff --git a/tests/tests/integration_tests.rs b/tests/tests/integration_tests.rs index 91df2841ee..ae021eedc3 100644 --- a/tests/tests/integration_tests.rs +++ b/tests/tests/integration_tests.rs @@ -330,7 +330,7 @@ async fn http_making_more_requests_than_allowed_should_not_deadlock() { async fn https_works() { init_logger(); - let client = HttpClientBuilder::default().build("https://kusama-rpc.polkadot.io:443").unwrap(); + let client = HttpClientBuilder::default().build("https://kusama-rpc.polkadot.io").unwrap(); let response: String = client.request("system_chain", rpc_params![]).await.unwrap(); assert_eq!(&response, "Kusama"); } @@ -339,27 +339,11 @@ async fn https_works() { async fn wss_works() { init_logger(); - let client = WsClientBuilder::default().build("wss://kusama-rpc.polkadot.io:443").await.unwrap(); + let client = WsClientBuilder::default().build("wss://kusama-rpc.polkadot.io").await.unwrap(); let response: String = client.request("system_chain", rpc_params![]).await.unwrap(); assert_eq!(&response, "Kusama"); } -#[tokio::test] -async fn ws_with_non_ascii_url_doesnt_hang_or_panic() { - init_logger(); - - let err = WsClientBuilder::default().build("wss://♥♥♥♥♥♥∀∂").await; - assert!(matches!(err, Err(Error::Transport(_)))); -} - -#[tokio::test] -async fn http_with_non_ascii_url_doesnt_hang_or_panic() { - init_logger(); - - let err = HttpClientBuilder::default().build("http://♥♥♥♥♥♥∀∂"); - assert!(matches!(err, Err(Error::Transport(_)))); -} - #[tokio::test] async fn ws_unsubscribe_releases_request_slots() { init_logger(); From f2271f0eb7c037292e3cd143e228cc8cfdb83f2c Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 9 Aug 2023 11:02:30 +0200 Subject: [PATCH 2/4] fix nits --- client/transport/src/ws/mod.rs | 51 ++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/client/transport/src/ws/mod.rs b/client/transport/src/ws/mod.rs index a26d81a934..c2a4c6bec8 100644 --- a/client/transport/src/ws/mod.rs +++ b/client/transport/src/ws/mod.rs @@ -496,15 +496,11 @@ impl TryFrom for Target { #[cfg(feature = "__tls")] let err = format!("`{invalid_scheme}` not supported, expects 'ws' or 'wss'"); #[cfg(not(feature = "__tls"))] - let err = format!("`{}` not supported, expects 'ws' ('wss' requires the tls feature)", scheme); + let err = format!("`{invalid_scheme}` not supported, expects 'ws' ('wss' requires the tls feature)"); return Err(WsHandshakeError::Url(err.into())); } }; - let host = - url.host_str().map(ToOwned::to_owned).ok_or_else(|| WsHandshakeError::Url("No host in URL".into()))?; - let port = url.port_or_known_default().ok_or_else(|| WsHandshakeError::Url("Invalid port in URL".into()))?; - - let host_header = if port == 80 || port == 443 { host.clone() } else { format!("{host}:{port}") }; + let host = url.host_str().map(ToOwned::to_owned).ok_or_else(|| WsHandshakeError::Url("Invalid host".into()))?; let mut path_and_query = url.path().to_owned(); if let Some(query) = url.query() { @@ -513,7 +509,13 @@ impl TryFrom for Target { } let sockaddrs = url.socket_addrs(|| None).map_err(WsHandshakeError::ResolutionFailed)?; - Ok(Self { sockaddrs, host, host_header, _mode, path_and_query: path_and_query.to_string() }) + Ok(Self { + sockaddrs, + host, + host_header: url.authority().to_string(), + _mode, + path_and_query: path_and_query.to_string(), + }) } } @@ -575,28 +577,22 @@ mod tests { } #[test] - fn ws_works() { + fn ws_works_with_port() { let target = parse_target("ws://127.0.0.1:9933").unwrap(); assert_ws_target(target, "127.0.0.1", "127.0.0.1:9933", Mode::Plain, "/"); } - #[test] - fn ws_default_port_works() { - let target = parse_target("ws://127.0.0.1").unwrap(); - assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/"); - } - #[cfg(feature = "__tls")] #[test] - fn wss_works() { - let target = parse_target("wss://kusama-rpc.polkadot.io").unwrap(); - assert_ws_target(target, "kusama-rpc.polkadot.io", "kusama-rpc.polkadot.io", Mode::Tls, "/"); + fn wss_works_with_port() { + let target = parse_target("wss://kusama-rpc.polkadot.io:9999").unwrap(); + assert_ws_target(target, "kusama-rpc.polkadot.io", "kusama-rpc.polkadot.io:9999", Mode::Tls, "/"); } #[cfg(not(feature = "__tls"))] #[test] fn wss_fails_with_tls_feature() { - let err = parse_target("wss://kusama-rpc.polkadot.io:443").unwrap_err(); + let err = parse_target("wss://kusama-rpc.polkadot.io").unwrap_err(); assert!(matches!(err, WsHandshakeError::Url(_))); } @@ -616,19 +612,32 @@ mod tests { #[test] fn url_with_path_works() { - let target = parse_target("ws://127.0.0.1:443/my-special-path").unwrap(); + let target = parse_target("ws://127.0.0.1/my-special-path").unwrap(); assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/my-special-path"); } #[test] fn url_with_query_works() { - let target = parse_target("ws://127.0.0.1:443/my?name1=value1&name2=value2").unwrap(); + let target = parse_target("ws://127.0.0.1/my?name1=value1&name2=value2").unwrap(); assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/my?name1=value1&name2=value2"); } #[test] fn url_with_fragment_is_ignored() { - let target = parse_target("ws://127.0.0.1:443/my.htm#ignore").unwrap(); + let target = parse_target("ws://127.0.0.1:/my.htm#ignore").unwrap(); assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/my.htm"); } + + #[cfg(feature = "__tls")] + #[test] + fn wss_default_port_is_omitted() { + let target = parse_target("wss://127.0.0.1:443").unwrap(); + assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Tls, "/"); + } + + #[test] + fn ws_default_port_is_omitted() { + let target = parse_target("ws://127.0.0.1:80").unwrap(); + assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/"); + } } From e9de68b400c966d1a39b431cb5fc0d0f60334d1e Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 9 Aug 2023 11:04:48 +0200 Subject: [PATCH 3/4] Update client/transport/src/ws/mod.rs --- client/transport/src/ws/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/transport/src/ws/mod.rs b/client/transport/src/ws/mod.rs index c2a4c6bec8..327c7323ac 100644 --- a/client/transport/src/ws/mod.rs +++ b/client/transport/src/ws/mod.rs @@ -349,7 +349,7 @@ impl WsTransportClientBuilder { } Ok(ServerResponse::Redirect { status_code, location }) => { tracing::debug!("Redirection: status_code: {}, location: {}", status_code, location); - match url::Url::parse(&location) { + match Url::parse(&location) { // redirection with absolute path => need to lookup. Ok(uri) => { // Absolute URI. From 68bdbbda92ab596cd727770a2f351f3833a6d90e Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 9 Aug 2023 11:36:42 +0200 Subject: [PATCH 4/4] fix more nits --- client/http-client/src/transport.rs | 51 +++++++++++++++++++++++------ client/transport/src/ws/mod.rs | 5 ++- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/client/http-client/src/transport.rs b/client/http-client/src/transport.rs index 2b618299cc..e38f4e55a6 100644 --- a/client/http-client/src/transport.rs +++ b/client/http-client/src/transport.rs @@ -280,7 +280,7 @@ mod tests { fn https_works() { let client = HttpTransportClient::new( 80, - "https://localhost:9933", + "https://localhost", 80, CertificateStore::Native, 80, @@ -288,7 +288,7 @@ mod tests { tower::ServiceBuilder::new(), ) .unwrap(); - assert_eq!(&client.target, "https://localhost:9933/"); + assert_eq!(&client.target, "https://localhost/"); } #[cfg(not(feature = "__tls"))] @@ -337,7 +337,7 @@ mod tests { fn url_with_path_works() { let client = HttpTransportClient::new( 1337, - "http://localhost:9944/my-special-path", + "http://localhost/my-special-path", 1337, CertificateStore::Native, 80, @@ -345,14 +345,14 @@ mod tests { tower::ServiceBuilder::new(), ) .unwrap(); - assert_eq!(&client.target, "http://localhost:9944/my-special-path"); + assert_eq!(&client.target, "http://localhost/my-special-path"); } #[test] fn url_with_query_works() { let client = HttpTransportClient::new( u32::MAX, - "http://127.0.0.1:9999/my?name1=value1&name2=value2", + "http://127.0.0.1/my?name1=value1&name2=value2", u32::MAX, CertificateStore::Native, 80, @@ -360,14 +360,14 @@ mod tests { tower::ServiceBuilder::new(), ) .unwrap(); - assert_eq!(&client.target, "http://127.0.0.1:9999/my?name1=value1&name2=value2"); + assert_eq!(&client.target, "http://127.0.0.1/my?name1=value1&name2=value2"); } #[test] fn url_with_fragment_is_ignored() { let client = HttpTransportClient::new( 999, - "http://127.0.0.1:9944/my.htm#ignore", + "http://127.0.0.1/my.htm#ignore", 999, CertificateStore::Native, 80, @@ -375,14 +375,14 @@ mod tests { tower::ServiceBuilder::new(), ) .unwrap(); - assert_eq!(&client.target, "http://127.0.0.1:9944/my.htm"); + assert_eq!(&client.target, "http://127.0.0.1/my.htm"); } #[test] - fn url_default_port_works() { + fn url_default_port_is_omitted() { let client = HttpTransportClient::new( 999, - "http://127.0.0.1", + "http://127.0.0.1:80", 999, CertificateStore::Native, 80, @@ -393,6 +393,37 @@ mod tests { assert_eq!(&client.target, "http://127.0.0.1/"); } + #[cfg(feature = "__tls")] + #[test] + fn https_custom_port_works() { + let client = HttpTransportClient::new( + 80, + "https://localhost:9999", + 80, + CertificateStore::Native, + 80, + HeaderMap::new(), + tower::ServiceBuilder::new(), + ) + .unwrap(); + assert_eq!(&client.target, "https://localhost:9999/"); + } + + #[test] + fn http_custom_port_works() { + let client = HttpTransportClient::new( + 80, + "http://localhost:9999", + 80, + CertificateStore::Native, + 80, + HeaderMap::new(), + tower::ServiceBuilder::new(), + ) + .unwrap(); + assert_eq!(&client.target, "http://localhost:9999/"); + } + #[tokio::test] async fn request_limit_works() { let eighty_bytes_limit = 80; diff --git a/client/transport/src/ws/mod.rs b/client/transport/src/ws/mod.rs index c2a4c6bec8..f48caabce6 100644 --- a/client/transport/src/ws/mod.rs +++ b/client/transport/src/ws/mod.rs @@ -372,9 +372,8 @@ impl WsTransportClientBuilder { }; } - // Relative URI. - Err(url::ParseError::RelativeUrlWithoutBase) - | Err(url::ParseError::RelativeUrlWithCannotBeABaseBase) => { + // Relative URI such as `/foo/bar.html` or `cake.html` + Err(url::ParseError::RelativeUrlWithoutBase) => { // Replace the entire path_and_query if `location` starts with `/` or `//`. if location.starts_with('/') { target.path_and_query = location;