From 1ae68efd82178aee9187d8924045a3c2e18de5e6 Mon Sep 17 00:00:00 2001 From: Fabrice Reix Date: Sun, 9 Oct 2022 16:30:07 +0200 Subject: [PATCH] Fix relative redirect --- integration/tests_ok/redirect.curl | 5 +- integration/tests_ok/redirect.html | 32 ++++++++++-- integration/tests_ok/redirect.hurl | 30 +++++++++-- integration/tests_ok/redirect.json | 2 +- integration/tests_ok/redirect.py | 16 ++++-- packages/hurl/src/http/client.rs | 30 +++++++++-- packages/hurl/src/http/error.rs | 1 + packages/hurl/src/http/request.rs | 83 +++++++++++++++++++++++++----- packages/hurl/src/runner/error.rs | 1 + packages/hurl/tests/libcurl.rs | 16 ++++-- 10 files changed, 181 insertions(+), 35 deletions(-) diff --git a/integration/tests_ok/redirect.curl b/integration/tests_ok/redirect.curl index 836c0e317..1e9cf5f46 100644 --- a/integration/tests_ok/redirect.curl +++ b/integration/tests_ok/redirect.curl @@ -1,2 +1,5 @@ -curl 'http://localhost:8000/redirect' curl 'http://localhost:8000/redirected' +curl 'http://localhost:8000/redirect-absolute' +curl 'http://localhost:8000/redirect-absolute' -L +curl 'http://localhost:8000/redirect-relative' +curl 'http://localhost:8000/redirect-relative' -L diff --git a/integration/tests_ok/redirect.html b/integration/tests_ok/redirect.html index 614073c3c..485a5c0a7 100644 --- a/integration/tests_ok/redirect.html +++ b/integration/tests_ok/redirect.html @@ -1,13 +1,37 @@ -
GET http://localhost:8000/redirect
+
GET http://localhost:8000/redirected
+HTTP/1.0 200
+```Redirected```
+
+
+# Absolute redirects
+
+GET http://localhost:8000/redirect-absolute
 HTTP/1.0 302
 Location: http://localhost:8000/redirected
 
-GET http://localhost:8000/redirected
-HTTP/1.0 200
+GET http://localhost:8000/redirect-absolute
+[Options]
+location: true
+
+HTTP/1.0 200
+```Redirected```
+
+
+# Relative redirects
+
+GET http://localhost:8000/redirect-relative
+HTTP/1.0 302
+Location: /redirected
+
+GET http://localhost:8000/redirect-relative
+[Options]
+location: true
+
+HTTP/1.0 200
+```Redirected```
 
 
 
 
 
-
 

\ No newline at end of file
diff --git a/integration/tests_ok/redirect.hurl b/integration/tests_ok/redirect.hurl
index 708ed94d4..e345be36f 100644
--- a/integration/tests_ok/redirect.hurl
+++ b/integration/tests_ok/redirect.hurl
@@ -1,10 +1,34 @@
-GET http://localhost:8000/redirect
+GET http://localhost:8000/redirected
+HTTP/1.0 200
+```Redirected```
+
+
+# Absolute redirects
+
+GET http://localhost:8000/redirect-absolute
 HTTP/1.0 302
 Location: http://localhost:8000/redirected
 
-GET http://localhost:8000/redirected
+GET http://localhost:8000/redirect-absolute
+[Options]
+location: true
+
 HTTP/1.0 200
-
+```Redirected```
+
+
+# Relative redirects
+
+GET http://localhost:8000/redirect-relative
+HTTP/1.0 302
+Location: /redirected
+
+GET http://localhost:8000/redirect-relative
+[Options]
+location: true
+
+HTTP/1.0 200
+```Redirected```
 
 
 
diff --git a/integration/tests_ok/redirect.json b/integration/tests_ok/redirect.json
index 8f5eaae97..2cf1f4f6b 100644
--- a/integration/tests_ok/redirect.json
+++ b/integration/tests_ok/redirect.json
@@ -1 +1 @@
-{"entries":[{"request":{"method":"GET","url":"http://localhost:8000/redirect"},"response":{"version":"HTTP/1.0","status":302,"headers":[{"name":"Location","value":"http://localhost:8000/redirected"}]}},{"request":{"method":"GET","url":"http://localhost:8000/redirected"},"response":{"version":"HTTP/1.0","status":200}}]}
\ No newline at end of file
+{"entries":[{"request":{"method":"GET","url":"http://localhost:8000/redirected"},"response":{"version":"HTTP/1.0","status":200,"body":{"type":"raw-string","value":"Redirected"}}},{"request":{"method":"GET","url":"http://localhost:8000/redirect-absolute"},"response":{"version":"HTTP/1.0","status":302,"headers":[{"name":"Location","value":"http://localhost:8000/redirected"}]}},{"request":{"method":"GET","url":"http://localhost:8000/redirect-absolute"},"response":{"version":"HTTP/1.0","status":200,"body":{"type":"raw-string","value":"Redirected"}}},{"request":{"method":"GET","url":"http://localhost:8000/redirect-relative"},"response":{"version":"HTTP/1.0","status":302,"headers":[{"name":"Location","value":"/redirected"}]}},{"request":{"method":"GET","url":"http://localhost:8000/redirect-relative"},"response":{"version":"HTTP/1.0","status":200,"body":{"type":"raw-string","value":"Redirected"}}}]}
\ No newline at end of file
diff --git a/integration/tests_ok/redirect.py b/integration/tests_ok/redirect.py
index e58d0b08b..89c7d0b10 100644
--- a/integration/tests_ok/redirect.py
+++ b/integration/tests_ok/redirect.py
@@ -1,12 +1,20 @@
 from app import app
-from flask import redirect
+from flask import redirect, Response
 
 
-@app.route("/redirect")
-def redirectme():
+@app.route("/redirect-absolute")
+def redirect_absolute():
     return redirect("http://localhost:8000/redirected")
 
 
+@app.route("/redirect-relative")
+def redirect_relative():
+    response = Response(status=302)
+    response.headers["Location"] = "/redirected"
+    response.autocorrect_location_header = False
+    return response
+
+
 @app.route("/redirected")
 def redirected():
-    return ""
+    return "Redirected"
diff --git a/packages/hurl/src/http/client.rs b/packages/hurl/src/http/client.rs
index 71ce9726f..741646c57 100644
--- a/packages/hurl/src/http/client.rs
+++ b/packages/hurl/src/http/client.rs
@@ -82,12 +82,13 @@ impl Client {
         self.redirect_count = 0;
         loop {
             let (request, response) = self.execute(&request_spec, options, logger)?;
-            calls.push((request, response.clone()));
+            calls.push((request.clone(), response.clone()));
             if !options.follow_location {
                 break;
             }
 
-            if let Some(url) = self.get_follow_location(&response) {
+            let base_url = request.base_url()?;
+            if let Some(url) = self.get_follow_location(&response, &base_url) {
                 logger.debug("");
                 logger.debug(format!("=> Redirect to {}", url).as_str());
                 logger.debug("");
@@ -513,14 +514,14 @@ 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) -> Option {
+    fn get_follow_location(&mut self, response: &Response, base_url: &str) -> Option {
         let response_code = response.status;
         if !(300..400).contains(&response_code) {
             return None;
         }
         let location = match response.get_header_values("Location").get(0) {
             None => return None,
-            Some(value) => value.clone(),
+            Some(value) => get_redirect_url(value, base_url),
         };
 
         if location.is_empty() {
@@ -590,6 +591,15 @@ 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 cookies from both cookies from the cookie storage and the request.
 pub fn all_cookies(cookie_storage: &[Cookie], request_spec: &RequestSpec) -> Vec {
     let mut cookies = request_spec.cookies.clone();
@@ -739,4 +749,16 @@ mod tests {
         assert!(match_cookie(&cookie, "http://sub.example.com/toto"));
         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()
+        );
+    }
 }
diff --git a/packages/hurl/src/http/error.rs b/packages/hurl/src/http/error.rs
index e8dc64fff..24d6fb6ec 100644
--- a/packages/hurl/src/http/error.rs
+++ b/packages/hurl/src/http/error.rs
@@ -40,4 +40,5 @@ pub enum HttpError {
     UnsupportedContentEncoding {
         description: String,
     },
+    InvalidUrl(String),
 }
diff --git a/packages/hurl/src/http/request.rs b/packages/hurl/src/http/request.rs
index f94d09194..0bc56adcf 100644
--- a/packages/hurl/src/http/request.rs
+++ b/packages/hurl/src/http/request.rs
@@ -18,7 +18,7 @@
 
 use super::core::*;
 use super::Header;
-use crate::http::header;
+use crate::http::{header, HttpError};
 use url::Url;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -61,6 +61,26 @@ impl Request {
             .get(0)
             .cloned()
     }
+
+    /// Returns the base url http(s)://host(:port)
+    pub fn base_url(&self) -> Result {
+        // FIXME: is it possible to do it with libcurl?
+        let url = match Url::parse(&self.url) {
+            Ok(url) => url,
+            Err(_) => return Err(HttpError::InvalidUrl(self.url.clone())),
+        };
+        let base_url = format!(
+            "{}://{}{}",
+            url.scheme(),
+            url.host().unwrap(),
+            if let Some(port) = url.port() {
+                format!(":{}", port)
+            } else {
+                "".to_string()
+            }
+        );
+        Ok(base_url)
+    }
 }
 
 fn parse_cookies(s: &str) -> Vec {
@@ -136,20 +156,20 @@ pub mod tests {
             vec![
                 Param {
                     name: "param1".to_string(),
-                    value: "value1".to_string()
+                    value: "value1".to_string(),
                 },
                 Param {
                     name: "param2".to_string(),
-                    value: "".to_string()
+                    value: "".to_string(),
                 },
                 Param {
                     name: "param3".to_string(),
-                    value: "a=b".to_string()
+                    value: "a=b".to_string(),
                 },
                 Param {
                     name: "param4".to_string(),
-                    value: "1,2,3".to_string()
-                }
+                    value: "1,2,3".to_string(),
+                },
             ]
         )
     }
@@ -162,12 +182,12 @@ pub mod tests {
             vec![
                 RequestCookie {
                     name: "cookie1".to_string(),
-                    value: "value1".to_string()
+                    value: "value1".to_string(),
                 },
                 RequestCookie {
                     name: "cookie2".to_string(),
-                    value: "value2".to_string()
-                }
+                    value: "value2".to_string(),
+                },
             ]
         )
     }
@@ -179,12 +199,12 @@ pub mod tests {
             vec![
                 RequestCookie {
                     name: "cookie1".to_string(),
-                    value: "value1".to_string()
+                    value: "value1".to_string(),
                 },
                 RequestCookie {
                     name: "cookie2".to_string(),
-                    value: "value2".to_string()
-                }
+                    value: "value2".to_string(),
+                },
             ]
         )
     }
@@ -195,8 +215,45 @@ pub mod tests {
             parse_cookie("cookie1=value1"),
             RequestCookie {
                 name: "cookie1".to_string(),
-                value: "value1".to_string()
+                value: "value1".to_string(),
             },
         )
     }
+
+    #[test]
+    fn test_base_url() {
+        assert_eq!(
+            Request {
+                url: "http://localhost".to_string(),
+                method: "".to_string(),
+                headers: vec![],
+                body: vec![],
+            }
+            .base_url()
+            .unwrap(),
+            "http://localhost".to_string()
+        );
+        assert_eq!(
+            Request {
+                url: "http://localhost:8000/redirect-relative".to_string(),
+                method: "".to_string(),
+                headers: vec![],
+                body: vec![],
+            }
+            .base_url()
+            .unwrap(),
+            "http://localhost:8000".to_string()
+        );
+        assert_eq!(
+            Request {
+                url: "https://localhost:8000".to_string(),
+                method: "".to_string(),
+                headers: vec![],
+                body: vec![],
+            }
+            .base_url()
+            .unwrap(),
+            "https://localhost:8000".to_string()
+        );
+    }
 }
diff --git a/packages/hurl/src/runner/error.rs b/packages/hurl/src/runner/error.rs
index d47c33072..96d2ba02d 100644
--- a/packages/hurl/src/runner/error.rs
+++ b/packages/hurl/src/runner/error.rs
@@ -172,6 +172,7 @@ impl From for RunnerError {
             HttpError::UnsupportedContentEncoding { description } => {
                 RunnerError::UnsupportedContentEncoding(description)
             }
+            HttpError::InvalidUrl(url) => RunnerError::InvalidUrl(url),
         }
     }
 }
diff --git a/packages/hurl/tests/libcurl.rs b/packages/hurl/tests/libcurl.rs
index da6356f2a..5584820b4 100644
--- a/packages/hurl/tests/libcurl.rs
+++ b/packages/hurl/tests/libcurl.rs
@@ -358,13 +358,16 @@ fn test_form_params() {
 
 #[test]
 fn test_redirect() {
-    let request_spec = default_get_request("http://localhost:8000/redirect");
+    let request_spec = default_get_request("http://localhost:8000/redirect-absolute");
     let logger = Logger::new(false, false, "", "");
     let options = ClientOptions::default();
     let mut client = Client::new(None);
     let (request, response) = client.execute(&request_spec, &options, &logger).unwrap();
     assert_eq!(request.method, "GET".to_string());
-    assert_eq!(request.url, "http://localhost:8000/redirect".to_string());
+    assert_eq!(
+        request.url,
+        "http://localhost:8000/redirect-absolute".to_string()
+    );
     assert_eq!(request.headers.len(), 3);
 
     assert_eq!(response.status, 302);
@@ -377,7 +380,7 @@ fn test_redirect() {
 
 #[test]
 fn test_follow_location() {
-    let request_spec = default_get_request("http://localhost:8000/redirect");
+    let request_spec = default_get_request("http://localhost:8000/redirect-absolute");
     let logger = Logger::new(false, false, "", "");
     let options = ClientOptions {
         follow_location: true,
@@ -388,7 +391,7 @@ fn test_follow_location() {
     assert_eq!(options.curl_args(), vec!["-L".to_string()]);
     assert_eq!(
         client.curl_command_line(&request_spec, &context_dir, &options),
-        "curl 'http://localhost:8000/redirect' -L".to_string()
+        "curl 'http://localhost:8000/redirect-absolute' -L".to_string()
     );
 
     let calls = client
@@ -398,7 +401,10 @@ fn test_follow_location() {
 
     let (request1, response1) = calls.get(0).unwrap();
     assert_eq!(request1.method, "GET".to_string());
-    assert_eq!(request1.url, "http://localhost:8000/redirect".to_string());
+    assert_eq!(
+        request1.url,
+        "http://localhost:8000/redirect-absolute".to_string()
+    );
     assert_eq!(request1.headers.len(), 3);
     assert_eq!(response1.status, 302);
     assert!(response1.headers.contains(&Header {