fix: use Url base_url support for parsing

Author: JosephLenton

src/async_impl/client.rs

@@ -37,7 +37,7 @@ use crate::cookie;
 use crate::dns::hickory::HickoryDnsResolver;
 use crate::dns::{gai::GaiResolver, DnsResolverWithOverrides, DynResolver, Resolve};
 use crate::error;
-use crate::into_url::try_uri;
+use crate::into_url::{try_uri, IntoUrlSealed};
 use crate::redirect::{self, remove_sensitive_headers};
 #[cfg(feature = "__tls")]
 use crate::tls::{self, TlsBackend};
@@ -816,7 +816,7 @@ impl ClientBuilder {
     /// base url has been set.
     pub fn base_url<U>(mut self, base_url: U) -> ClientBuilder
     where
-        U: IntoUrl
+        U: IntoUrl,
     {
         match base_url.into_url() {
             Ok(base_url) => {
@@ -1994,25 +1994,14 @@ impl Client {
     ///
     /// This method fails whenever the supplied `Url` cannot be parsed.
     pub fn request<U: IntoUrl>(&self, method: Method, url: U) -> RequestBuilder {
-        let mut maybe_url = url.into_url();
-
-        if let Some(base_url) = &self.inner.base_url {
-            if let Err(url_parse_error) = &maybe_url {
-                if url_parse_error.is_builder() {
-                    if let Some(url) = url_parse_error.url() {
-                        if !url.has_host() {
-                            let mut replacement_url = base_url.clone();
-
-                            replacement_url.set_path(url.path());
-                            replacement_url.set_query(url.query());
-                            replacement_url.set_fragment(url.fragment());
-
-                            maybe_url = Ok(replacement_url);
-                        }
-                    }
-                }
-            }
-        }
+        let maybe_url = match &self.inner.base_url {
+            Some(ref base_url) => Url::options()
+                .base_url(Some(base_url))
+                .parse(url.as_str())
+                .map_err(crate::error::builder)
+                .and_then(IntoUrlSealed::into_url),
+            None => url.into_url(),
+        };
 
         let req = maybe_url.map(move |url| Request::new(method, url));
         RequestBuilder::new(self.clone(), req)

tests/client.rs

@@ -574,32 +574,59 @@ async fn highly_concurrent_requests_to_slow_http2_server_with_low_max_concurrent
 }
 
 #[test]
-fn client_cannot_be_built_with_non_absolute_url() {
+fn client_cannot_be_built_with_relative_base_url() {
     let result = reqwest::Client::builder().base_url("/users/123").build();
 
     assert!(result.is_err());
 }
 
-/*
+#[test]
+fn client_can_be_built_with_absolute_base_url() {
+    let result = reqwest::Client::builder()
+        .base_url("http://example.com/users/123")
+        .build();
+
+    assert!(result.is_ok());
+}
+
 #[tokio::test]
-async fn todo_() {
+async fn client_with_base_url_accepts_relative_urls() {
     let _ = env_logger::builder().is_test(true).try_init();
     let server = server::http(move |_req| async { http::Response::new("Hello".into()) });
 
-    let url = format!(
-        "http://{overridden_domain}:{}/domain_override",
-        server.addr().port()
-    );
+    let server_base_url = format!("http://{}", server.addr());
     let client = reqwest::Client::builder()
+        .base_url(server_base_url)
         .no_proxy()
-        .resolve(overridden_domain, server.addr())
         .build()
         .expect("client builder");
-    let req = client.get(&url);
+
+    // use a relative url on request
+    let req = client.get("/hello");
+    let res = req.send().await.expect("request");
+
+    assert_eq!(res.status(), reqwest::StatusCode::OK);
+    let text = res.text().await.expect("Failed to get text");
+    assert_eq!("Hello", text);
+}
+
+#[tokio::test]
+async fn client_with_base_url_accepts_absolute_urls() {
+    let _ = env_logger::builder().is_test(true).try_init();
+    let server = server::http(move |_req| async { http::Response::new("Hello".into()) });
+
+    let client = reqwest::Client::builder()
+        .base_url("http://example.com")
+        .no_proxy()
+        .build()
+        .expect("client builder");
+
+    // use an absolute url on request
+    let request_absolute_url = format!("http://{}/hello", server.addr());
+    let req = client.get(request_absolute_url);
     let res = req.send().await.expect("request");
 
     assert_eq!(res.status(), reqwest::StatusCode::OK);
     let text = res.text().await.expect("Failed to get text");
     assert_eq!("Hello", text);
 }
- */