Refacto using our own Url struct

Orange-OpenSource · Oct 31, 2024 · 64b93a5 · 64b93a5
1 parent 36203a3
commit 64b93a5
Show file tree

Hide file tree

Showing 7 changed files with 163 additions and 65 deletions.
diff --git a/integration/hurl/tests_failed/invalid_url.err.pattern b/integration/hurl/tests_failed/invalid_url.err.pattern
@@ -1,7 +1,7 @@
-error: HTTP connection
+error: Invalid URL
   --> tests_failed/invalid_url.hurl:1:5
    |
  1 | GET https://???
-   |     ^^^^^^^^^^^ (3) <<<.*?>>>
+   |     ^^^^^^^^^^^ invalid URL <https://???> (empty host)
    |
 
diff --git a/packages/hurl/src/http/client.rs b/packages/hurl/src/http/client.rs
@@ -150,7 +150,7 @@ impl Client {
             }
             request_spec = RequestSpec {
                 method: redirect_method,
-                url: redirect_url.to_string(),
+                url: redirect_url,
                 headers,
                 ..Default::default()
             };
@@ -501,12 +501,13 @@ impl Client {
     }
 
     /// Generates URL.
-    fn generate_url(&mut self, url: &str, params: &[Param]) -> String {
+    fn generate_url(&mut self, url: &Url, params: &[Param]) -> String {
+        let url = url.raw();
         if params.is_empty() {
-            url.to_string()
+            url
         } else {
             let url = if url.ends_with('?') {
-                url.to_string()
+                url
             } else if url.contains('?') {
                 format!("{url}&")
             } else {
@@ -852,7 +853,7 @@ pub fn all_cookies(cookie_storage: &[Cookie], request_spec: &RequestSpec) -> Vec
         &mut cookie_storage
             .iter()
             .filter(|c| c.expires != "1") // cookie expired when libcurl set value to 1?
-            .filter(|c| match_cookie(c, request_spec.url.as_str()))
+            .filter(|c| match_cookie(c, &request_spec.url))
             .map(|c| RequestCookie {
                 name: c.name.clone(),
                 value: c.value.clone(),
@@ -863,12 +864,7 @@ pub fn all_cookies(cookie_storage: &[Cookie], request_spec: &RequestSpec) -> Vec
 }
 
 /// Matches cookie for a given URL.
-pub fn match_cookie(cookie: &Cookie, url: &str) -> bool {
-    // FIXME: is it possible to do it with libcurl?
-    let url = match url::Url::parse(url) {
-        Ok(url) => url,
-        Err(_) => return false,
-    };
+pub fn match_cookie(cookie: &Cookie, url: &Url) -> bool {
     if let Some(domain) = url.domain() {
         if cookie.include_subdomain == "FALSE" {
             if cookie.domain != domain {
@@ -1045,9 +1041,18 @@ mod tests {
             value: String::new(),
             http_only: false,
         };
-        assert!(match_cookie(&cookie, "http://example.com/toto"));
-        assert!(!match_cookie(&cookie, "http://sub.example.com/tata"));
-        assert!(!match_cookie(&cookie, "http://toto/tata"));
+        assert!(match_cookie(
+            &cookie,
+            &Url::from_str("http://example.com/toto").unwrap()
+        ));
+        assert!(!match_cookie(
+            &cookie,
+            &Url::from_str("http://sub.example.com/tata").unwrap()
+        ));
+        assert!(!match_cookie(
+            &cookie,
+            &Url::from_str("http://toto/tata").unwrap()
+        ));
 
         let cookie = Cookie {
             domain: "example.com".to_string(),
@@ -1059,9 +1064,18 @@ mod tests {
             value: String::new(),
             http_only: false,
         };
-        assert!(match_cookie(&cookie, "http://example.com/toto"));
-        assert!(match_cookie(&cookie, "http://sub.example.com/toto"));
-        assert!(!match_cookie(&cookie, "http://example.com/tata"));
+        assert!(match_cookie(
+            &cookie,
+            &Url::from_str("http://example.com/toto").unwrap()
+        ));
+        assert!(match_cookie(
+            &cookie,
+            &Url::from_str("http://sub.example.com/toto").unwrap()
+        ));
+        assert!(!match_cookie(
+            &cookie,
+            &Url::from_str("http://example.com/tata").unwrap()
+        ));
     }
 
     #[test]
@@ -1157,7 +1171,7 @@ mod tests {
         let mut client = Client::new();
         let request = RequestSpec {
             method: Method("GET".to_string()),
-            url: "https://example.org".to_string(),
+            url: Url::from_str("https://example.org").unwrap(),
             ..Default::default()
         };
         let context_dir = ContextDir::default();

diff --git a/packages/hurl/src/http/request_spec.rs b/packages/hurl/src/http/request_spec.rs
@@ -19,13 +19,14 @@ use core::fmt;
 
 use crate::http::core::*;
 use crate::http::header::HeaderVec;
+use crate::http::Url;
 
 /// Represents the HTTP request asked to be executed by our user (different from the runtime
 /// executed HTTP request [`crate::http::Request`].
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct RequestSpec {
     pub method: Method,
-    pub url: String,
+    pub url: Url,
     pub headers: HeaderVec,
     pub querystring: Vec<Param>,
     pub form: Vec<Param>,
@@ -43,7 +44,7 @@ impl Default for RequestSpec {
     fn default() -> Self {
         RequestSpec {
             method: Method("GET".to_string()),
-            url: String::new(),
+            url: Url::default(),
             headers: HeaderVec::new(),
             querystring: vec![],
             form: vec![],

diff --git a/packages/hurl/src/http/request_spec_curl_args.rs b/packages/hurl/src/http/request_spec_curl_args.rs
@@ -109,11 +109,11 @@ impl RequestSpec {
             params.join("&")
         };
         let url = if querystring.as_str() == "" {
-            self.url.to_string()
-        } else if self.url.to_string().contains('?') {
-            format!("{}&{}", self.url, querystring)
+            self.url.raw()
+        } else if self.url.raw().contains('?') {
+            format!("{}&{}", self.url.raw(), querystring)
         } else {
-            format!("{}?{}", self.url, querystring)
+            format!("{}?{}", self.url.raw(), querystring)
         };
         arguments.push(format!("'{url}'"));
 
@@ -253,6 +253,7 @@ fn escape_string(s: &str) -> String {
 #[cfg(test)]
 pub mod tests {
     use std::path::Path;
+    use std::str::FromStr;
 
     use super::*;
 
@@ -265,7 +266,7 @@ pub mod tests {
 
         RequestSpec {
             method: Method("POST".to_string()),
-            url: "http://localhost/form-params".to_string(),
+            url: Url::from_str("http://localhost/form-params").unwrap(),
             headers,
             form: vec![
                 Param {
@@ -287,7 +288,7 @@ pub mod tests {
         headers.push(Header::new("content-type", "application/vnd.api+json"));
         RequestSpec {
             method: Method("POST".to_string()),
-            url: "http://localhost/json".to_string(),
+            url: Url::from_str("http://localhost/json").unwrap(),
             headers,
             body: Body::Text("{\"foo\":\"bar\"}".to_string()),
             implicit_content_type: Some("application/json".to_string()),
@@ -421,14 +422,24 @@ pub mod tests {
                 "'http://localhost/json'".to_string(),
             ]
         );
+
+        assert_eq!(
+            RequestSpec {
+                method: Method("GET".to_string()),
+                url: Url::from_str("http://localhost:8000/").unwrap(),
+                ..Default::default()
+            }
+            .curl_args(context_dir),
+            vec!["'http://localhost:8000/'".to_string(),]
+        );
     }
 
     #[test]
     fn post_data_curl_args() {
         let context_dir = &ContextDir::default();
         let req = RequestSpec {
             method: Method("POST".to_string()),
-            url: "http://localhost:8000/hello".to_string(),
+            url: Url::from_str("http://localhost:8000/hello").unwrap(),
             body: Body::Text("foo".to_string()),
             ..Default::default()
         };
@@ -446,7 +457,7 @@ pub mod tests {
         let context_dir = &ContextDir::default();
         let req = RequestSpec {
             method: Method("POST".to_string()),
-            url: "http://localhost:8000/hello".to_string(),
+            url: Url::from_str("http://localhost:8000/hello").unwrap(),
             body: Body::File(b"Hello World!".to_vec(), "foo.bin".to_string()),
             ..Default::default()
         };

diff --git a/packages/hurl/src/http/tests/mod.rs b/packages/hurl/src/http/tests/mod.rs
@@ -38,7 +38,7 @@ fn default_response() -> Response {
 pub fn hello_http_request() -> RequestSpec {
     RequestSpec {
         method: Method("GET".to_string()),
-        url: "http://localhost:8000/hello".to_string(),
+        url: Url::from_str("http://localhost:8000/hello").unwrap(),
         ..Default::default()
     }
 }
@@ -145,7 +145,7 @@ pub fn html_http_response() -> Response {
 pub fn query_http_request() -> RequestSpec {
     RequestSpec {
         method: Method("GET".to_string()),
-        url: "http://localhost:8000/querystring-params".to_string(),
+        url: Url::from_str("http://localhost:8000/querystring-params").unwrap(),
         querystring: vec![
             Param {
                 name: String::from("param1"),
@@ -167,7 +167,7 @@ pub fn custom_http_request() -> RequestSpec {
 
     RequestSpec {
         method: Method("GET".to_string()),
-        url: "http://localhost/custom".to_string(),
+        url: Url::from_str("http://localhost/custom").unwrap(),
         headers,
         cookies: vec![
             RequestCookie {