Skip to content

Commit

Permalink
refactor(client): support default port number (#1172)
Browse files Browse the repository at this point in the history 
* refactor(client): support default port number

* fix nits

* Update client/transport/src/ws/mod.rs

* fix more nits
  • Loading branch information
@niklasad1
niklasad1 committed Aug 9, 2023
1 parent 38193f0 commit ad9dab3
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 143 deletions.
1 change: 1 addition & 0 deletions client/http-client/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
113 changes: 66 additions & 47 deletions client/http-client/src/transport.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";

Expand Down Expand Up @@ -77,7 +77,7 @@ where
#[derive(Debug, Clone)]
pub struct HttpTransportClient<S> {
/// Target to connect to.
target: ParsedUri,
target: String,
/// HTTP client
client: S,
/// Configurable max request body size
Expand Down Expand Up @@ -109,13 +109,17 @@ where
headers: HeaderMap,
service_builder: tower::ServiceBuilder<L>,
) -> Result<Self, Error> {
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()
Expand Down Expand Up @@ -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,
Expand All @@ -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();
}
Expand Down Expand Up @@ -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<Self, Self::Error> {
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 {
Expand Down Expand Up @@ -272,21 +260,6 @@ mod tests {
use super::*;
use jsonrpsee_core::client::CertificateStore;

fn assert_target(
client: &HttpTransportClient<HttpBackend>,
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(
Expand All @@ -307,15 +280,15 @@ mod tests {
fn https_works() {
let client = HttpTransportClient::new(
80,
"https://localhost:9933",
"https://localhost",
80,
CertificateStore::Native,
80,
HeaderMap::new(),
tower::ServiceBuilder::new(),
)
.unwrap();
assert_target(&client, "localhost", "https", "/", 9933, 80);
assert_eq!(&client.target, "https://localhost/");
}

#[cfg(not(feature = "__tls"))]
Expand Down Expand Up @@ -364,45 +337,91 @@ 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,
HeaderMap::new(),
tower::ServiceBuilder::new(),
)
.unwrap();
assert_target(&client, "localhost", "http", "/my-special-path", 9944, 1337);
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,
HeaderMap::new(),
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/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,
HeaderMap::new(),
tower::ServiceBuilder::new(),
)
.unwrap();
assert_eq!(&client.target, "http://127.0.0.1/my.htm");
}

#[test]
fn url_default_port_is_omitted() {
let client = HttpTransportClient::new(
999,
"http://127.0.0.1:80",
999,
CertificateStore::Native,
80,
HeaderMap::new(),
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/");
}

#[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]
Expand Down
2 changes: 2 additions & 0 deletions client/transport/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -53,6 +54,7 @@ ws = [
"soketto",
"pin-project",
"thiserror",
"url",
]
web = [
"gloo-net",
Expand Down
Loading

0 comments on commit ad9dab3

Please sign in to comment.