Fix creation of redirect URL.

packages/hurl/src/http/client.rs

@@ -114,8 +114,7 @@ impl Client {
         let mut redirect_count = 0;
         loop {
             let call = self.execute(&request_spec, options, logger)?;
-            let base_url = call.request.base_url()?;
-            let redirect_url = self.get_follow_location(&call.response, &base_url);
+            let redirect_url = self.get_follow_location(&call.request, &call.response)?;
             let status = call.response.status;
             calls.push(call);
             if !options.follow_location || redirect_url.is_none() {
@@ -140,7 +139,7 @@ impl Client {
             };
             request_spec = RequestSpec {
                 method: redirect_method,
-                url: redirect_url,
+                url: redirect_url.to_string(),
                 headers,
                 ..Default::default()
             };
@@ -684,15 +683,20 @@ impl Client {
     /// 1. the option follow_location set to true
     /// 2. a 3xx response code
     /// 3. a header Location
-    fn get_follow_location(&mut self, response: &Response, base_url: &str) -> Option<String> {
+    fn get_follow_location(
+        &mut self,
+        request: &Request,
+        response: &Response,
+    ) -> Result<Option<Url>, HttpError> {
         let response_code = response.status;
         if !(300..400).contains(&response_code) {
-            return None;
+            return Ok(None);
         }
-        response
-            .headers
-            .get(LOCATION)
-            .map(|h| get_redirect_url(&h.value, base_url))
+        let Some(location) = response.headers.get(LOCATION) else {
+            return Ok(None);
+        };
+        let url = request.url.join(&location.value)?;
+        Ok(Some(url))
     }
 
     /// Returns cookie storage.
@@ -776,15 +780,6 @@ impl Client {
     }
 }
 
-/// Returns the redirect url.
-fn get_redirect_url(location: &str, base_url: &str) -> String {
-    if location.starts_with('/') {
-        format!("{base_url}{location}")
-    } else {
-        location.to_string()
-    }
-}
-
 /// Returns the method used for redirecting a request/response with `response_status`.
 fn get_redirect_method(response_status: u32, original_method: Method) -> Method {
     // This replicates curl's behavior
@@ -1014,18 +1009,6 @@ mod tests {
         assert!(!match_cookie(&cookie, "http://example.com/tata"));
     }
 
-    #[test]
-    fn test_redirect_url() {
-        assert_eq!(
-            get_redirect_url("http://localhost:8000/redirected", "http://localhost:8000"),
-            "http://localhost:8000/redirected".to_string()
-        );
-        assert_eq!(
-            get_redirect_url("/redirected", "http://localhost:8000"),
-            "http://localhost:8000/redirected".to_string()
-        );
-    }
-
     #[test]
     fn test_redirect_method() {
         // Status of the response to be redirected | method of the original request | method of the new request

packages/hurl/src/http/request.rs

@@ -20,7 +20,6 @@ use std::fmt;
 use crate::http::core::*;
 use crate::http::header::{HeaderVec, COOKIE};
 use crate::http::url::Url;
-use crate::http::HttpError;
 
 /// Represents a runtime HTTP request.
 /// This is a real request, that has been executed by our HTTP client.
@@ -91,11 +90,6 @@ impl Request {
             .flat_map(|h| parse_cookies(h.value.as_str().trim()))
             .collect()
     }
-
-    /// Returns the base url http(s)://host(:port)
-    pub fn base_url(&self) -> Result<String, HttpError> {
-        self.url.base_url()
-    }
 }
 
 fn parse_cookies(s: &str) -> Vec<RequestCookie> {
@@ -199,41 +193,4 @@ mod tests {
             },
         );
     }
-
-    #[test]
-    fn test_base_url() {
-        assert_eq!(
-            Request::new(
-                "",
-                Url::try_from("http://localhost").unwrap(),
-                HeaderVec::new(),
-                vec![]
-            )
-            .base_url()
-            .unwrap(),
-            "http://localhost".to_string()
-        );
-        assert_eq!(
-            Request::new(
-                "",
-                Url::try_from("http://localhost:8000/redirect-relative").unwrap(),
-                HeaderVec::new(),
-                vec![]
-            )
-            .base_url()
-            .unwrap(),
-            "http://localhost:8000".to_string()
-        );
-        assert_eq!(
-            Request::new(
-                "",
-                Url::try_from("https://localhost:8000").unwrap(),
-                HeaderVec::new(),
-                vec![]
-            )
-            .base_url()
-            .unwrap(),
-            "https://localhost:8000".to_string()
-        );
-    }
 }

packages/hurl/src/http/url.rs

@@ -34,29 +34,19 @@ impl Url {
             .collect()
     }
 
-    /// TODO: Temporary method, will be deleted soon
-    pub fn base_url(&self) -> Result<String, HttpError> {
-        let scheme = self.inner.scheme();
-        if scheme != "http" && scheme != "https" {
-            return Err(HttpError::InvalidUrl(
-                self.inner.to_string(),
-                "Missing protocol http or https".to_string(),
-            ));
-        }
-        let host = match self.inner.host() {
-            Some(host) => host,
-            None => {
+    /// Parse a string `input` as an URL, with this URL as the base URL.
+    pub fn join(&self, input: &str) -> Result<Url, HttpError> {
+        let new_inner = self.inner.join(input);
+        let new_inner = match new_inner {
+            Ok(u) => u,
+            Err(_) => {
                 return Err(HttpError::InvalidUrl(
                     self.inner.to_string(),
-                    "Can not extract host".to_string(),
+                    format!("Can not use relative path '{input}'"),
                 ))
             }
         };
-        let port = match self.inner.port() {
-            Some(port) => format!(":{port}"),
-            None => String::new(),
-        };
-        Ok(format!("{scheme}://{host}{port}"))
+        Url::try_from(new_inner.to_string().as_str())
     }
 }
 
@@ -121,4 +111,32 @@ mod tests {
             ]
         );
     }
+
+    #[test]
+    fn test_join() {
+        let base = Url::try_from("http://example.net/foo/index.html").unwrap();
+
+        // Test join with absolute
+        assert_eq!(
+            base.join("http://bar.com/redirected").unwrap(),
+            Url::try_from("http://bar.com/redirected").unwrap()
+        );
+
+        // Test join with relative
+        assert_eq!(
+            base.join("/redirected").unwrap(),
+            Url::try_from("http://example.net/redirected").unwrap()
+        );
+
+        assert_eq!(
+            base.join("../bar/index.html").unwrap(),
+            Url::try_from("http://example.net/bar/index.html").unwrap()
+        );
+
+        // Scheme relative URL
+        assert_eq!(
+            base.join("//example.org/baz/index.html").unwrap(),
+            Url::try_from("http://example.org/baz/index.html").unwrap()
+        )
+    }
 }