public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v3] client/http_client: add necessary brackets
@ 2021-05-05  8:36 Dietmar Maurer
  2021-05-05  9:07 ` Dominik Csapak
  2021-05-05  9:09 ` [pbs-devel] applied: " Dietmar Maurer
  0 siblings, 2 replies; 3+ messages in thread
From: Dietmar Maurer @ 2021-05-05  8:36 UTC (permalink / raw)
  To: pbs-devel

if we are given a 'naked' ipv6 without square brackets around it,
we need to add them ourselves, since the address is ambigious otherwise
when we add the port.

e.g. giving 'fe80::1' as address we arrive at the url (with the default port)
'https://fe80::1:8007/'

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---

changes from v2:
* add build_uri() method which returns an Uri
* try to avoid unnecessary allocation (result in some code duplication)

changes from v1:
* move the actual mapping to the request building functions

 src/client/http_client.rs | 106 +++++++++++++++++++++-----------------
 1 file changed, 58 insertions(+), 48 deletions(-)

diff --git a/src/client/http_client.rs b/src/client/http_client.rs
index 76ab0391..f2fada23 100644
--- a/src/client/http_client.rs
+++ b/src/client/http_client.rs
@@ -273,6 +273,26 @@ fn load_ticket_info(prefix: &str, server: &str, userid: &Userid) -> Option<(Stri
     }
 }
 
+fn build_uri(server: &str, port: u16, path: &str, query: Option<String>) -> Result<Uri, Error> {
+    let path = path.trim_matches('/');
+    let bytes = server.as_bytes();
+    let len = bytes.len();
+    let uri = if len > 3 && bytes.contains(&b':') && bytes[0] != b'[' && bytes[len-1] != b']' {
+        if let Some(query) = query {
+            format!("https://[{}]:{}/{}?{}", server, port, path, query)
+        } else {
+            format!("https://[{}]:{}/{}", server, port, path)
+        }
+    } else {
+        if let Some(query) = query {
+            format!("https://{}:{}/{}?{}", server, port, path, query)
+        } else {
+            format!("https://{}:{}/{}", server, port, path)
+        }
+    };
+    Ok(uri.parse()?)
+}
+
 impl HttpClient {
     pub fn new(
         server: &str,
@@ -614,16 +634,11 @@ impl HttpClient {
         data: Option<Value>,
     ) -> Result<Value, Error> {
 
-        let path = path.trim_matches('/');
-        let mut url = format!("https://{}:{}/{}", &self.server, self.port, path);
-
-        if let Some(data) = data {
-            let query = tools::json_object_to_query(data).unwrap();
-            url.push('?');
-            url.push_str(&query);
-        }
-
-        let url: Uri = url.parse().unwrap();
+        let query = match data {
+            Some(data) => Some(tools::json_object_to_query(data)?),
+            None => None,
+        };
+        let url = build_uri(&self.server, self.port, path, query)?;
 
         let req = Request::builder()
             .method("POST")
@@ -757,39 +772,38 @@ impl HttpClient {
     }
 
     pub fn request_builder(server: &str, port: u16, method: &str, path: &str, data: Option<Value>) -> Result<Request<Body>, Error> {
-        let path = path.trim_matches('/');
-        let url: Uri = format!("https://{}:{}/{}", server, port, path).parse()?;
-
         if let Some(data) = data {
             if method == "POST" {
+                let url = build_uri(server, port, path, None)?;
                 let request = Request::builder()
                     .method(method)
                     .uri(url)
                     .header("User-Agent", "proxmox-backup-client/1.0")
                     .header(hyper::header::CONTENT_TYPE, "application/json")
                     .body(Body::from(data.to_string()))?;
-                return Ok(request);
+                Ok(request)
             } else {
                 let query = tools::json_object_to_query(data)?;
-                let url: Uri = format!("https://{}:{}/{}?{}", server, port, path, query).parse()?;
+                let url = build_uri(server, port, path, Some(query))?;
                 let request = Request::builder()
                     .method(method)
                     .uri(url)
                     .header("User-Agent", "proxmox-backup-client/1.0")
                     .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
                     .body(Body::empty())?;
-                return Ok(request);
+                Ok(request)
             }
-        }
-
-        let request = Request::builder()
-            .method(method)
-            .uri(url)
-            .header("User-Agent", "proxmox-backup-client/1.0")
-            .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
-            .body(Body::empty())?;
+        } else {
+            let url = build_uri(server, port, path, None)?;
+            let request = Request::builder()
+                .method(method)
+                .uri(url)
+                .header("User-Agent", "proxmox-backup-client/1.0")
+                .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
+                .body(Body::empty())?;
 
-        Ok(request)
+            Ok(request)
+        }
     }
 }
 
@@ -970,29 +984,25 @@ impl H2Client {
         let path = path.trim_matches('/');
 
         let content_type = content_type.unwrap_or("application/x-www-form-urlencoded");
+        let query = match param {
+            Some(param) => {
+                let query = tools::json_object_to_query(param)?;
+                // We detected problem with hyper around 6000 characters - so we try to keep on the safe side
+                if query.len() > 4096 {
+                    bail!("h2 query data too large ({} bytes) - please encode data inside body", query.len());
+                }
+                Some(query)
+            }
+            None => None,
+        };
 
-        if let Some(param) = param {
-            let query = tools::json_object_to_query(param)?;
-            // We detected problem with hyper around 6000 characters - seo we try to keep on the safe side
-            if query.len() > 4096 { bail!("h2 query data too large ({} bytes) - please encode data inside body", query.len()); }
-            let url: Uri = format!("https://{}:8007/{}?{}", server, path, query).parse()?;
-             let request = Request::builder()
-                .method(method)
-                .uri(url)
-                .header("User-Agent", "proxmox-backup-client/1.0")
-                .header(hyper::header::CONTENT_TYPE, content_type)
-                .body(())?;
-            Ok(request)
-        } else {
-            let url: Uri = format!("https://{}:8007/{}", server, path).parse()?;
-            let request = Request::builder()
-                .method(method)
-                .uri(url)
-                .header("User-Agent", "proxmox-backup-client/1.0")
-                .header(hyper::header::CONTENT_TYPE, content_type)
-                .body(())?;
-
-            Ok(request)
-        }
+        let url = build_uri(server, 8007, path, query)?;
+        let request = Request::builder()
+            .method(method)
+            .uri(url)
+            .header("User-Agent", "proxmox-backup-client/1.0")
+            .header(hyper::header::CONTENT_TYPE, content_type)
+            .body(())?;
+        Ok(request)
     }
 }
-- 
2.20.1




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v3] client/http_client: add necessary brackets
  2021-05-05  8:36 [pbs-devel] [PATCH proxmox-backup v3] client/http_client: add necessary brackets Dietmar Maurer
@ 2021-05-05  9:07 ` Dominik Csapak
  2021-05-05  9:09 ` [pbs-devel] applied: " Dietmar Maurer
  1 sibling, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2021-05-05  9:07 UTC (permalink / raw)
  To: Dietmar Maurer, pbs-devel

yes, looks way better than my approach

On 5/5/21 10:36, Dietmar Maurer wrote:
> if we are given a 'naked' ipv6 without square brackets around it,
> we need to add them ourselves, since the address is ambigious otherwise
> when we add the port.
> 
> e.g. giving 'fe80::1' as address we arrive at the url (with the default port)
> 'https://fe80::1:8007/'
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
> 
> changes from v2:
> * add build_uri() method which returns an Uri
> * try to avoid unnecessary allocation (result in some code duplication)
> 
> changes from v1:
> * move the actual mapping to the request building functions
> 
>   src/client/http_client.rs | 106 +++++++++++++++++++++-----------------
>   1 file changed, 58 insertions(+), 48 deletions(-)
> 
> diff --git a/src/client/http_client.rs b/src/client/http_client.rs
> index 76ab0391..f2fada23 100644
> --- a/src/client/http_client.rs
> +++ b/src/client/http_client.rs
> @@ -273,6 +273,26 @@ fn load_ticket_info(prefix: &str, server: &str, userid: &Userid) -> Option<(Stri
>       }
>   }
>   
> +fn build_uri(server: &str, port: u16, path: &str, query: Option<String>) -> Result<Uri, Error> {
> +    let path = path.trim_matches('/');
> +    let bytes = server.as_bytes();
> +    let len = bytes.len();
> +    let uri = if len > 3 && bytes.contains(&b':') && bytes[0] != b'[' && bytes[len-1] != b']' {
> +        if let Some(query) = query {
> +            format!("https://[{}]:{}/{}?{}", server, port, path, query)
> +        } else {
> +            format!("https://[{}]:{}/{}", server, port, path)
> +        }
> +    } else {
> +        if let Some(query) = query {
> +            format!("https://{}:{}/{}?{}", server, port, path, query)
> +        } else {
> +            format!("https://{}:{}/{}", server, port, path)
> +        }
> +    };
> +    Ok(uri.parse()?)
> +}
> +
>   impl HttpClient {
>       pub fn new(
>           server: &str,
> @@ -614,16 +634,11 @@ impl HttpClient {
>           data: Option<Value>,
>       ) -> Result<Value, Error> {
>   
> -        let path = path.trim_matches('/');
> -        let mut url = format!("https://{}:{}/{}", &self.server, self.port, path);
> -
> -        if let Some(data) = data {
> -            let query = tools::json_object_to_query(data).unwrap();
> -            url.push('?');
> -            url.push_str(&query);
> -        }
> -
> -        let url: Uri = url.parse().unwrap();
> +        let query = match data {
> +            Some(data) => Some(tools::json_object_to_query(data)?),
> +            None => None,
> +        };
> +        let url = build_uri(&self.server, self.port, path, query)?;
>   
>           let req = Request::builder()
>               .method("POST")
> @@ -757,39 +772,38 @@ impl HttpClient {
>       }
>   
>       pub fn request_builder(server: &str, port: u16, method: &str, path: &str, data: Option<Value>) -> Result<Request<Body>, Error> {
> -        let path = path.trim_matches('/');
> -        let url: Uri = format!("https://{}:{}/{}", server, port, path).parse()?;
> -
>           if let Some(data) = data {
>               if method == "POST" {
> +                let url = build_uri(server, port, path, None)?;
>                   let request = Request::builder()
>                       .method(method)
>                       .uri(url)
>                       .header("User-Agent", "proxmox-backup-client/1.0")
>                       .header(hyper::header::CONTENT_TYPE, "application/json")
>                       .body(Body::from(data.to_string()))?;
> -                return Ok(request);
> +                Ok(request)
>               } else {
>                   let query = tools::json_object_to_query(data)?;
> -                let url: Uri = format!("https://{}:{}/{}?{}", server, port, path, query).parse()?;
> +                let url = build_uri(server, port, path, Some(query))?;
>                   let request = Request::builder()
>                       .method(method)
>                       .uri(url)
>                       .header("User-Agent", "proxmox-backup-client/1.0")
>                       .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
>                       .body(Body::empty())?;
> -                return Ok(request);
> +                Ok(request)
>               }
> -        }
> -
> -        let request = Request::builder()
> -            .method(method)
> -            .uri(url)
> -            .header("User-Agent", "proxmox-backup-client/1.0")
> -            .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
> -            .body(Body::empty())?;
> +        } else {
> +            let url = build_uri(server, port, path, None)?;
> +            let request = Request::builder()
> +                .method(method)
> +                .uri(url)
> +                .header("User-Agent", "proxmox-backup-client/1.0")
> +                .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
> +                .body(Body::empty())?;
>   
> -        Ok(request)
> +            Ok(request)
> +        }
>       }
>   }
>   
> @@ -970,29 +984,25 @@ impl H2Client {
>           let path = path.trim_matches('/');
>   
>           let content_type = content_type.unwrap_or("application/x-www-form-urlencoded");
> +        let query = match param {
> +            Some(param) => {
> +                let query = tools::json_object_to_query(param)?;
> +                // We detected problem with hyper around 6000 characters - so we try to keep on the safe side
> +                if query.len() > 4096 {
> +                    bail!("h2 query data too large ({} bytes) - please encode data inside body", query.len());
> +                }
> +                Some(query)
> +            }
> +            None => None,
> +        };
>   
> -        if let Some(param) = param {
> -            let query = tools::json_object_to_query(param)?;
> -            // We detected problem with hyper around 6000 characters - seo we try to keep on the safe side
> -            if query.len() > 4096 { bail!("h2 query data too large ({} bytes) - please encode data inside body", query.len()); }
> -            let url: Uri = format!("https://{}:8007/{}?{}", server, path, query).parse()?;
> -             let request = Request::builder()
> -                .method(method)
> -                .uri(url)
> -                .header("User-Agent", "proxmox-backup-client/1.0")
> -                .header(hyper::header::CONTENT_TYPE, content_type)
> -                .body(())?;
> -            Ok(request)
> -        } else {
> -            let url: Uri = format!("https://{}:8007/{}", server, path).parse()?;
> -            let request = Request::builder()
> -                .method(method)
> -                .uri(url)
> -                .header("User-Agent", "proxmox-backup-client/1.0")
> -                .header(hyper::header::CONTENT_TYPE, content_type)
> -                .body(())?;
> -
> -            Ok(request)
> -        }
> +        let url = build_uri(server, 8007, path, query)?;
> +        let request = Request::builder()
> +            .method(method)
> +            .uri(url)
> +            .header("User-Agent", "proxmox-backup-client/1.0")
> +            .header(hyper::header::CONTENT_TYPE, content_type)
> +            .body(())?;
> +        Ok(request)
>       }
>   }
> 





^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pbs-devel] applied: [PATCH proxmox-backup v3] client/http_client: add necessary brackets
  2021-05-05  8:36 [pbs-devel] [PATCH proxmox-backup v3] client/http_client: add necessary brackets Dietmar Maurer
  2021-05-05  9:07 ` Dominik Csapak
@ 2021-05-05  9:09 ` Dietmar Maurer
  1 sibling, 0 replies; 3+ messages in thread
From: Dietmar Maurer @ 2021-05-05  9:09 UTC (permalink / raw)
  To: pbs-devel

applied

On 5/5/21 10:36 AM, Dietmar Maurer wrote:
> if we are given a 'naked' ipv6 without square brackets around it,
> we need to add them ourselves, since the address is ambigious otherwise
> when we add the port.
>
> e.g. giving 'fe80::1' as address we arrive at the url (with the default port)
> 'https://fe80::1:8007/'
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
>
> changes from v2:
> * add build_uri() method which returns an Uri
> * try to avoid unnecessary allocation (result in some code duplication)
>
> changes from v1:
> * move the actual mapping to the request building functions
>
>   src/client/http_client.rs | 106 +++++++++++++++++++++-----------------
>   1 file changed, 58 insertions(+), 48 deletions(-)
>
> diff --git a/src/client/http_client.rs b/src/client/http_client.rs
> index 76ab0391..f2fada23 100644
> --- a/src/client/http_client.rs
> +++ b/src/client/http_client.rs
> @@ -273,6 +273,26 @@ fn load_ticket_info(prefix: &str, server: &str, userid: &Userid) -> Option<(Stri
>       }
>   }
>   
> +fn build_uri(server: &str, port: u16, path: &str, query: Option<String>) -> Result<Uri, Error> {
> +    let path = path.trim_matches('/');
> +    let bytes = server.as_bytes();
> +    let len = bytes.len();
> +    let uri = if len > 3 && bytes.contains(&b':') && bytes[0] != b'[' && bytes[len-1] != b']' {
> +        if let Some(query) = query {
> +            format!("https://[{}]:{}/{}?{}", server, port, path, query)
> +        } else {
> +            format!("https://[{}]:{}/{}", server, port, path)
> +        }
> +    } else {
> +        if let Some(query) = query {
> +            format!("https://{}:{}/{}?{}", server, port, path, query)
> +        } else {
> +            format!("https://{}:{}/{}", server, port, path)
> +        }
> +    };
> +    Ok(uri.parse()?)
> +}
> +
>   impl HttpClient {
>       pub fn new(
>           server: &str,
> @@ -614,16 +634,11 @@ impl HttpClient {
>           data: Option<Value>,
>       ) -> Result<Value, Error> {
>   
> -        let path = path.trim_matches('/');
> -        let mut url = format!("https://{}:{}/{}", &self.server, self.port, path);
> -
> -        if let Some(data) = data {
> -            let query = tools::json_object_to_query(data).unwrap();
> -            url.push('?');
> -            url.push_str(&query);
> -        }
> -
> -        let url: Uri = url.parse().unwrap();
> +        let query = match data {
> +            Some(data) => Some(tools::json_object_to_query(data)?),
> +            None => None,
> +        };
> +        let url = build_uri(&self.server, self.port, path, query)?;
>   
>           let req = Request::builder()
>               .method("POST")
> @@ -757,39 +772,38 @@ impl HttpClient {
>       }
>   
>       pub fn request_builder(server: &str, port: u16, method: &str, path: &str, data: Option<Value>) -> Result<Request<Body>, Error> {
> -        let path = path.trim_matches('/');
> -        let url: Uri = format!("https://{}:{}/{}", server, port, path).parse()?;
> -
>           if let Some(data) = data {
>               if method == "POST" {
> +                let url = build_uri(server, port, path, None)?;
>                   let request = Request::builder()
>                       .method(method)
>                       .uri(url)
>                       .header("User-Agent", "proxmox-backup-client/1.0")
>                       .header(hyper::header::CONTENT_TYPE, "application/json")
>                       .body(Body::from(data.to_string()))?;
> -                return Ok(request);
> +                Ok(request)
>               } else {
>                   let query = tools::json_object_to_query(data)?;
> -                let url: Uri = format!("https://{}:{}/{}?{}", server, port, path, query).parse()?;
> +                let url = build_uri(server, port, path, Some(query))?;
>                   let request = Request::builder()
>                       .method(method)
>                       .uri(url)
>                       .header("User-Agent", "proxmox-backup-client/1.0")
>                       .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
>                       .body(Body::empty())?;
> -                return Ok(request);
> +                Ok(request)
>               }
> -        }
> -
> -        let request = Request::builder()
> -            .method(method)
> -            .uri(url)
> -            .header("User-Agent", "proxmox-backup-client/1.0")
> -            .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
> -            .body(Body::empty())?;
> +        } else {
> +            let url = build_uri(server, port, path, None)?;
> +            let request = Request::builder()
> +                .method(method)
> +                .uri(url)
> +                .header("User-Agent", "proxmox-backup-client/1.0")
> +                .header(hyper::header::CONTENT_TYPE, "application/x-www-form-urlencoded")
> +                .body(Body::empty())?;
>   
> -        Ok(request)
> +            Ok(request)
> +        }
>       }
>   }
>   
> @@ -970,29 +984,25 @@ impl H2Client {
>           let path = path.trim_matches('/');
>   
>           let content_type = content_type.unwrap_or("application/x-www-form-urlencoded");
> +        let query = match param {
> +            Some(param) => {
> +                let query = tools::json_object_to_query(param)?;
> +                // We detected problem with hyper around 6000 characters - so we try to keep on the safe side
> +                if query.len() > 4096 {
> +                    bail!("h2 query data too large ({} bytes) - please encode data inside body", query.len());
> +                }
> +                Some(query)
> +            }
> +            None => None,
> +        };
>   
> -        if let Some(param) = param {
> -            let query = tools::json_object_to_query(param)?;
> -            // We detected problem with hyper around 6000 characters - seo we try to keep on the safe side
> -            if query.len() > 4096 { bail!("h2 query data too large ({} bytes) - please encode data inside body", query.len()); }
> -            let url: Uri = format!("https://{}:8007/{}?{}", server, path, query).parse()?;
> -             let request = Request::builder()
> -                .method(method)
> -                .uri(url)
> -                .header("User-Agent", "proxmox-backup-client/1.0")
> -                .header(hyper::header::CONTENT_TYPE, content_type)
> -                .body(())?;
> -            Ok(request)
> -        } else {
> -            let url: Uri = format!("https://{}:8007/{}", server, path).parse()?;
> -            let request = Request::builder()
> -                .method(method)
> -                .uri(url)
> -                .header("User-Agent", "proxmox-backup-client/1.0")
> -                .header(hyper::header::CONTENT_TYPE, content_type)
> -                .body(())?;
> -
> -            Ok(request)
> -        }
> +        let url = build_uri(server, 8007, path, query)?;
> +        let request = Request::builder()
> +            .method(method)
> +            .uri(url)
> +            .header("User-Agent", "proxmox-backup-client/1.0")
> +            .header(hyper::header::CONTENT_TYPE, content_type)
> +            .body(())?;
> +        Ok(request)
>       }
>   }




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-05  9:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-05  8:36 [pbs-devel] [PATCH proxmox-backup v3] client/http_client: add necessary brackets Dietmar Maurer
2021-05-05  9:07 ` Dominik Csapak
2021-05-05  9:09 ` [pbs-devel] applied: " Dietmar Maurer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal