all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] correctly set apt proxy configuration
@ 2021-05-11  6:54 Dietmar Maurer
  2021-05-12 10:01 ` Thomas Lamprecht
  0 siblings, 1 reply; 3+ messages in thread
From: Dietmar Maurer @ 2021-05-11  6:54 UTC (permalink / raw)
  To: pbs-devel

---
 src/api2/node/apt.rs    | 41 ++++++++++++++++++++++++++++++++---------
 src/api2/node/config.rs |  9 ++++++++-
 src/config/node.rs      |  4 +++-
 src/tools/http.rs       | 18 +++++++++++++++---
 4 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 1e57ea8d..83ccc460 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -5,11 +5,11 @@ use std::collections::HashMap;
 use proxmox::list_subdirs_api_method;
 use proxmox::api::{api, RpcEnvironment, RpcEnvironmentType, Permission};
 use proxmox::api::router::{Router, SubdirMap};
+use proxmox::tools::fs::{replace_file, CreateOptions};
 
 use crate::config::node;
 use crate::server::WorkerTask;
-use crate::tools::{apt, SimpleHttp, subscription};
-
+use crate::tools::{apt, SimpleHttp, http::ProxyConfig, subscription};
 use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
 use crate::api2::types::{Authid, APTUpdateInfo, NODE_SCHEMA, UPID_SCHEMA};
 
@@ -47,10 +47,36 @@ fn apt_update_available(_param: Value) -> Result<Value, Error> {
     Ok(json!(cache.package_status))
 }
 
+pub fn update_apt_proxy_config(proxy_config: Option<&ProxyConfig>) -> Result<(), Error> {
+
+    const PROXY_CFG_FN: &str = "/etc/apt/apt.conf.d/76pveproxy"; // use same file as PVE
+
+    if let Some(proxy_config) = proxy_config {
+        let proxy = proxy_config.to_proxy_string()?;
+        let data = format!("Acquire::http::Proxy \"{}\";\n", proxy);
+        replace_file(PROXY_CFG_FN, data.as_bytes(), CreateOptions::new())?;
+    } else {
+        let _ = std::fs::remove_file(PROXY_CFG_FN);
+    }
+
+    Ok(())
+}
+
+fn read_and_update_proxy_config() -> Result<Option<ProxyConfig>, Error> {
+    let proxy_config = if let Ok((node_config, _digest)) = node::config() {
+        node_config.http_proxy()
+    } else {
+        None
+    };
+    update_apt_proxy_config(proxy_config.as_ref())?;
+
+    Ok(proxy_config)
+}
+
 fn do_apt_update(worker: &WorkerTask, quiet: bool) -> Result<(), Error> {
     if !quiet { worker.log("starting apt-get update") }
 
-    // TODO: set proxy /etc/apt/apt.conf.d/76pbsproxy like PVE
+    read_and_update_proxy_config()?;
 
     let mut command = std::process::Command::new("apt-get");
     command.arg("update");
@@ -153,6 +179,7 @@ pub fn apt_update_database(
 }
 
 #[api(
+    protected: true,
     input: {
         properties: {
             node: {
@@ -195,12 +222,7 @@ fn apt_get_changelog(
         bail!("Package '{}' not found", name);
     }
 
-    let proxy_config = if let Ok((node_config, _digest)) = node::config() {
-        node_config.http_proxy()
-    } else {
-        None
-    };
-
+    let proxy_config = read_and_update_proxy_config()?;
     let mut client = SimpleHttp::new(proxy_config);
 
     let changelog_url = &pkg_info[0].change_log_url;
@@ -235,6 +257,7 @@ fn apt_get_changelog(
         Ok(json!(changelog))
 
     } else {
+        eprintln!("TZEST2");
         let mut command = std::process::Command::new("apt-get");
         command.arg("changelog");
         command.arg("-qq"); // don't display download progress
diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
index 92c0100e..290e9284 100644
--- a/src/api2/node/config.rs
+++ b/src/api2/node/config.rs
@@ -4,6 +4,7 @@ use proxmox::api::schema::Updatable;
 use proxmox::api::{api, Permission, Router, RpcEnvironment};
 
 use crate::api2::types::NODE_SCHEMA;
+use crate::api2::node::apt::update_apt_proxy_config;
 use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
 use crate::config::node::{NodeConfig, NodeConfigUpdater};
 
@@ -78,5 +79,11 @@ pub fn update_node_config(
 
     config.update_from(updater, &delete)?;
 
-    crate::config::node::save_config(&config)
+    let http_proxy = config.http_proxy();
+
+    crate::config::node::save_config(&config)?;
+
+    update_apt_proxy_config(http_proxy.as_ref())?;
+
+    Ok(())
 }
diff --git a/src/config/node.rs b/src/config/node.rs
index 31a2c5ab..33f3e2ad 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -151,6 +151,7 @@ impl NodeConfig {
         AcmeDomainIter::new(self)
     }
 
+    /// Returns the parsed ProxyConfig
     pub fn http_proxy(&self) -> Option<ProxyConfig> {
         if let Some(http_proxy) = &self.http_proxy {
             match ProxyConfig::parse_proxy_url(&http_proxy) {
@@ -162,7 +163,8 @@ impl NodeConfig {
         }
     }
 
-    pub fn set_proxy(&mut self, http_proxy: Option<String>) {
+    /// Sets the HTTP proxy configuration
+    pub fn set_http_proxy(&mut self, http_proxy: Option<String>) {
         self.http_proxy = http_proxy;
     }
 
diff --git a/src/tools/http.rs b/src/tools/http.rs
index 998e7647..0821992a 100644
--- a/src/tools/http.rs
+++ b/src/tools/http.rs
@@ -43,7 +43,7 @@ pub(crate) fn build_authority(host: &str, port: u16) -> Result<Authority, Error>
 pub struct ProxyConfig {
     pub host: String,
     pub port: u16,
-    pub authorization: Option<String>, // Proxy-Authorization header value
+    pub authorization: Option<String>, // user:pass
     pub force_connect: bool,
 }
 
@@ -94,7 +94,7 @@ impl ProxyConfig {
 
             let authority_vec: Vec<&str> = proxy_authority.as_str().rsplitn(2, '@').collect();
             let authorization = if authority_vec.len() == 2 {
-                Some(format!("Basic {}", base64::encode(authority_vec[1])))
+                Some(authority_vec[1].to_string())
             } else {
                 None
             };
@@ -107,6 +107,15 @@ impl ProxyConfig {
             })
         }).map_err(|err| format_err!("parse_proxy_url failed: {}", err))
     }
+
+    /// Assemble canonical proxy string (including scheme and port)
+    pub fn to_proxy_string(&self) -> Result<String, Error> {
+        let authority = build_authority(&self.host, self.port)?;
+        Ok(match self.authorization {
+            None => format!("http://{}", authority),
+            Some(ref authorization) => format!("http://{}@{}", authorization, authority)
+        })
+    }
 }
 
 #[derive(Clone)]
@@ -241,7 +250,10 @@ impl hyper::service::Service<Uri> for HttpsConnector {
 
                     let mut connect_request = format!("CONNECT {0}:{1} HTTP/1.1\r\n", host, port);
                     if let Some(authorization) = authorization {
-                        connect_request.push_str(&format!("Proxy-Authorization: {}\r\n", authorization));
+                        connect_request.push_str(&format!(
+                            "Proxy-Authorization: Basic {}\r\n",
+                            base64::encode(authorization),
+                        ));
                     }
                     connect_request.push_str(&format!("Host: {0}:{1}\r\n\r\n", host, port));
 
-- 
2.20.1




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

* Re: [pbs-devel] [PATCH proxmox-backup] correctly set apt proxy configuration
  2021-05-11  6:54 [pbs-devel] [PATCH proxmox-backup] correctly set apt proxy configuration Dietmar Maurer
@ 2021-05-12 10:01 ` Thomas Lamprecht
  2021-05-12 10:21   ` [pbs-devel] applied: " Dietmar Maurer
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Lamprecht @ 2021-05-12 10:01 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dietmar Maurer

looks mostly good, a few comments in line.

On 11.05.21 08:54, Dietmar Maurer wrote:
> ---
>  src/api2/node/apt.rs    | 41 ++++++++++++++++++++++++++++++++---------
>  src/api2/node/config.rs |  9 ++++++++-
>  src/config/node.rs      |  4 +++-
>  src/tools/http.rs       | 18 +++++++++++++++---
>  4 files changed, 58 insertions(+), 14 deletions(-)
> 
> diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
> index 1e57ea8d..83ccc460 100644
> --- a/src/api2/node/apt.rs
> +++ b/src/api2/node/apt.rs
> @@ -5,11 +5,11 @@ use std::collections::HashMap;
>  use proxmox::list_subdirs_api_method;
>  use proxmox::api::{api, RpcEnvironment, RpcEnvironmentType, Permission};
>  use proxmox::api::router::{Router, SubdirMap};
> +use proxmox::tools::fs::{replace_file, CreateOptions};
>  
>  use crate::config::node;
>  use crate::server::WorkerTask;
> -use crate::tools::{apt, SimpleHttp, subscription};
> -
> +use crate::tools::{apt, SimpleHttp, http::ProxyConfig, subscription};
>  use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
>  use crate::api2::types::{Authid, APTUpdateInfo, NODE_SCHEMA, UPID_SCHEMA};
>  
> @@ -47,10 +47,36 @@ fn apt_update_available(_param: Value) -> Result<Value, Error> {
>      Ok(json!(cache.package_status))
>  }
>  
> +pub fn update_apt_proxy_config(proxy_config: Option<&ProxyConfig>) -> Result<(), Error> {
> +
> +    const PROXY_CFG_FN: &str = "/etc/apt/apt.conf.d/76pveproxy"; // use same file as PVE
> +
> +    if let Some(proxy_config) = proxy_config {
> +        let proxy = proxy_config.to_proxy_string()?;
> +        let data = format!("Acquire::http::Proxy \"{}\";\n", proxy);
> +        replace_file(PROXY_CFG_FN, data.as_bytes(), CreateOptions::new())?;
> +    } else {
> +        let _ = std::fs::remove_file(PROXY_CFG_FN);


I do not like such results thrown away, I mean it is unlikely, but if the removal fails the
proxy would still be configured for apt, which may come as a surprise for an user.

Can we do something like:

std::fs::remove_file(PROXY_CFG_FN).map_err(|err| {
    format_err!("failed to remove proxy config '{}' - {}", PROXY_CFG_FN, err)
})?;

here?

> +    }
> +
> +    Ok(())
> +}
> +
> +fn read_and_update_proxy_config() -> Result<Option<ProxyConfig>, Error> {
> +    let proxy_config = if let Ok((node_config, _digest)) = node::config() {
> +        node_config.http_proxy()
> +    } else {
> +        None
> +    };
> +    update_apt_proxy_config(proxy_config.as_ref())?;
> +
> +    Ok(proxy_config)
> +}
> +
>  fn do_apt_update(worker: &WorkerTask, quiet: bool) -> Result<(), Error> {
>      if !quiet { worker.log("starting apt-get update") }
>  
> -    // TODO: set proxy /etc/apt/apt.conf.d/76pbsproxy like PVE
> +    read_and_update_proxy_config()?;
>  
>      let mut command = std::process::Command::new("apt-get");
>      command.arg("update");
> @@ -153,6 +179,7 @@ pub fn apt_update_database(
>  }
>  
>  #[api(
> +    protected: true,
>      input: {
>          properties: {
>              node: {
> @@ -195,12 +222,7 @@ fn apt_get_changelog(
>          bail!("Package '{}' not found", name);
>      }
>  
> -    let proxy_config = if let Ok((node_config, _digest)) = node::config() {
> -        node_config.http_proxy()
> -    } else {
> -        None
> -    };
> -
> +    let proxy_config = read_and_update_proxy_config()?;
>      let mut client = SimpleHttp::new(proxy_config);
>  
>      let changelog_url = &pkg_info[0].change_log_url;
> @@ -235,6 +257,7 @@ fn apt_get_changelog(
>          Ok(json!(changelog))
>  
>      } else {
> +        eprintln!("TZEST2");


please remove the left over debug print above


>          let mut command = std::process::Command::new("apt-get");
>          command.arg("changelog");
>          command.arg("-qq"); // don't display download progress
> diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
> index 92c0100e..290e9284 100644
> --- a/src/api2/node/config.rs
> +++ b/src/api2/node/config.rs
> @@ -4,6 +4,7 @@ use proxmox::api::schema::Updatable;
>  use proxmox::api::{api, Permission, Router, RpcEnvironment};
>  
>  use crate::api2::types::NODE_SCHEMA;
> +use crate::api2::node::apt::update_apt_proxy_config;
>  use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
>  use crate::config::node::{NodeConfig, NodeConfigUpdater};
>  
> @@ -78,5 +79,11 @@ pub fn update_node_config(
>  
>      config.update_from(updater, &delete)?;
>  
> -    crate::config::node::save_config(&config)
> +    let http_proxy = config.http_proxy();
> +
> +    crate::config::node::save_config(&config)?;
> +
> +    update_apt_proxy_config(http_proxy.as_ref())?;

would be enough and IMO still readable if we'd just that in one line here:

update_apt_proxy_config(config.http_proxy().as_ref())?;

but no hard feelings here

> +
> +    Ok(())
>  }
> diff --git a/src/config/node.rs b/src/config/node.rs
> index 31a2c5ab..33f3e2ad 100644
> --- a/src/config/node.rs
> +++ b/src/config/node.rs
> @@ -151,6 +151,7 @@ impl NodeConfig {
>          AcmeDomainIter::new(self)
>      }
>  
> +    /// Returns the parsed ProxyConfig
>      pub fn http_proxy(&self) -> Option<ProxyConfig> {
>          if let Some(http_proxy) = &self.http_proxy {
>              match ProxyConfig::parse_proxy_url(&http_proxy) {
> @@ -162,7 +163,8 @@ impl NodeConfig {
>          }
>      }
>  
> -    pub fn set_proxy(&mut self, http_proxy: Option<String>) {
> +    /// Sets the HTTP proxy configuration
> +    pub fn set_http_proxy(&mut self, http_proxy: Option<String>) {
>          self.http_proxy = http_proxy;
>      }
>  
> diff --git a/src/tools/http.rs b/src/tools/http.rs
> index 998e7647..0821992a 100644
> --- a/src/tools/http.rs
> +++ b/src/tools/http.rs
> @@ -43,7 +43,7 @@ pub(crate) fn build_authority(host: &str, port: u16) -> Result<Authority, Error>
>  pub struct ProxyConfig {
>      pub host: String,
>      pub port: u16,
> -    pub authorization: Option<String>, // Proxy-Authorization header value
> +    pub authorization: Option<String>, // user:pass
>      pub force_connect: bool,
>  }
>  
> @@ -94,7 +94,7 @@ impl ProxyConfig {
>  
>              let authority_vec: Vec<&str> = proxy_authority.as_str().rsplitn(2, '@').collect();
>              let authorization = if authority_vec.len() == 2 {
> -                Some(format!("Basic {}", base64::encode(authority_vec[1])))
> +                Some(authority_vec[1].to_string())

It seems like all changes to this file and config/node.rs could haven been its own patch,
as they are not directly related to setting the apt proxy, but I'm fine with it as is too.

>              } else {
>                  None
>              };
> @@ -107,6 +107,15 @@ impl ProxyConfig {
>              })
>          }).map_err(|err| format_err!("parse_proxy_url failed: {}", err))
>      }
> +
> +    /// Assemble canonical proxy string (including scheme and port)
> +    pub fn to_proxy_string(&self) -> Result<String, Error> {
> +        let authority = build_authority(&self.host, self.port)?;
> +        Ok(match self.authorization {
> +            None => format!("http://{}", authority),
> +            Some(ref authorization) => format!("http://{}@{}", authorization, authority)
> +        })
> +    }
>  }
>  
>  #[derive(Clone)]
> @@ -241,7 +250,10 @@ impl hyper::service::Service<Uri> for HttpsConnector {
>  
>                      let mut connect_request = format!("CONNECT {0}:{1} HTTP/1.1\r\n", host, port);
>                      if let Some(authorization) = authorization {
> -                        connect_request.push_str(&format!("Proxy-Authorization: {}\r\n", authorization));
> +                        connect_request.push_str(&format!(
> +                            "Proxy-Authorization: Basic {}\r\n",
> +                            base64::encode(authorization),
> +                        ));
>                      }
>                      connect_request.push_str(&format!("Host: {0}:{1}\r\n\r\n", host, port));
>  
> 





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

* [pbs-devel] applied: [PATCH proxmox-backup] correctly set apt proxy configuration
  2021-05-12 10:01 ` Thomas Lamprecht
@ 2021-05-12 10:21   ` Dietmar Maurer
  0 siblings, 0 replies; 3+ messages in thread
From: Dietmar Maurer @ 2021-05-12 10:21 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox Backup Server development discussion

applied with suggested changes


On 5/12/21 12:01 PM, Thomas Lamprecht wrote:
> looks mostly good, a few comments in line.
>
> On 11.05.21 08:54, Dietmar Maurer wrote:
>> ---
>>   src/api2/node/apt.rs    | 41 ++++++++++++++++++++++++++++++++---------
>>   src/api2/node/config.rs |  9 ++++++++-
>>   src/config/node.rs      |  4 +++-
>>   src/tools/http.rs       | 18 +++++++++++++++---
>>   4 files changed, 58 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
>> index 1e57ea8d..83ccc460 100644
>> --- a/src/api2/node/apt.rs
>> +++ b/src/api2/node/apt.rs
>> @@ -5,11 +5,11 @@ use std::collections::HashMap;
>>   use proxmox::list_subdirs_api_method;
>>   use proxmox::api::{api, RpcEnvironment, RpcEnvironmentType, Permission};
>>   use proxmox::api::router::{Router, SubdirMap};
>> +use proxmox::tools::fs::{replace_file, CreateOptions};
>>   
>>   use crate::config::node;
>>   use crate::server::WorkerTask;
>> -use crate::tools::{apt, SimpleHttp, subscription};
>> -
>> +use crate::tools::{apt, SimpleHttp, http::ProxyConfig, subscription};
>>   use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
>>   use crate::api2::types::{Authid, APTUpdateInfo, NODE_SCHEMA, UPID_SCHEMA};
>>   
>> @@ -47,10 +47,36 @@ fn apt_update_available(_param: Value) -> Result<Value, Error> {
>>       Ok(json!(cache.package_status))
>>   }
>>   
>> +pub fn update_apt_proxy_config(proxy_config: Option<&ProxyConfig>) -> Result<(), Error> {
>> +
>> +    const PROXY_CFG_FN: &str = "/etc/apt/apt.conf.d/76pveproxy"; // use same file as PVE
>> +
>> +    if let Some(proxy_config) = proxy_config {
>> +        let proxy = proxy_config.to_proxy_string()?;
>> +        let data = format!("Acquire::http::Proxy \"{}\";\n", proxy);
>> +        replace_file(PROXY_CFG_FN, data.as_bytes(), CreateOptions::new())?;
>> +    } else {
>> +        let _ = std::fs::remove_file(PROXY_CFG_FN);
>
> I do not like such results thrown away, I mean it is unlikely, but if the removal fails the
> proxy would still be configured for apt, which may come as a surprise for an user.
>
> Can we do something like:
>
> std::fs::remove_file(PROXY_CFG_FN).map_err(|err| {
>      format_err!("failed to remove proxy config '{}' - {}", PROXY_CFG_FN, err)
> })?;
>
> here?
>
>> +    }
>> +
>> +    Ok(())
>> +}
>> +
>> +fn read_and_update_proxy_config() -> Result<Option<ProxyConfig>, Error> {
>> +    let proxy_config = if let Ok((node_config, _digest)) = node::config() {
>> +        node_config.http_proxy()
>> +    } else {
>> +        None
>> +    };
>> +    update_apt_proxy_config(proxy_config.as_ref())?;
>> +
>> +    Ok(proxy_config)
>> +}
>> +
>>   fn do_apt_update(worker: &WorkerTask, quiet: bool) -> Result<(), Error> {
>>       if !quiet { worker.log("starting apt-get update") }
>>   
>> -    // TODO: set proxy /etc/apt/apt.conf.d/76pbsproxy like PVE
>> +    read_and_update_proxy_config()?;
>>   
>>       let mut command = std::process::Command::new("apt-get");
>>       command.arg("update");
>> @@ -153,6 +179,7 @@ pub fn apt_update_database(
>>   }
>>   
>>   #[api(
>> +    protected: true,
>>       input: {
>>           properties: {
>>               node: {
>> @@ -195,12 +222,7 @@ fn apt_get_changelog(
>>           bail!("Package '{}' not found", name);
>>       }
>>   
>> -    let proxy_config = if let Ok((node_config, _digest)) = node::config() {
>> -        node_config.http_proxy()
>> -    } else {
>> -        None
>> -    };
>> -
>> +    let proxy_config = read_and_update_proxy_config()?;
>>       let mut client = SimpleHttp::new(proxy_config);
>>   
>>       let changelog_url = &pkg_info[0].change_log_url;
>> @@ -235,6 +257,7 @@ fn apt_get_changelog(
>>           Ok(json!(changelog))
>>   
>>       } else {
>> +        eprintln!("TZEST2");
>
> please remove the left over debug print above
>
>
>>           let mut command = std::process::Command::new("apt-get");
>>           command.arg("changelog");
>>           command.arg("-qq"); // don't display download progress
>> diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
>> index 92c0100e..290e9284 100644
>> --- a/src/api2/node/config.rs
>> +++ b/src/api2/node/config.rs
>> @@ -4,6 +4,7 @@ use proxmox::api::schema::Updatable;
>>   use proxmox::api::{api, Permission, Router, RpcEnvironment};
>>   
>>   use crate::api2::types::NODE_SCHEMA;
>> +use crate::api2::node::apt::update_apt_proxy_config;
>>   use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
>>   use crate::config::node::{NodeConfig, NodeConfigUpdater};
>>   
>> @@ -78,5 +79,11 @@ pub fn update_node_config(
>>   
>>       config.update_from(updater, &delete)?;
>>   
>> -    crate::config::node::save_config(&config)
>> +    let http_proxy = config.http_proxy();
>> +
>> +    crate::config::node::save_config(&config)?;
>> +
>> +    update_apt_proxy_config(http_proxy.as_ref())?;
> would be enough and IMO still readable if we'd just that in one line here:
>
> update_apt_proxy_config(config.http_proxy().as_ref())?;
>
> but no hard feelings here
>
>> +
>> +    Ok(())
>>   }
>> diff --git a/src/config/node.rs b/src/config/node.rs
>> index 31a2c5ab..33f3e2ad 100644
>> --- a/src/config/node.rs
>> +++ b/src/config/node.rs
>> @@ -151,6 +151,7 @@ impl NodeConfig {
>>           AcmeDomainIter::new(self)
>>       }
>>   
>> +    /// Returns the parsed ProxyConfig
>>       pub fn http_proxy(&self) -> Option<ProxyConfig> {
>>           if let Some(http_proxy) = &self.http_proxy {
>>               match ProxyConfig::parse_proxy_url(&http_proxy) {
>> @@ -162,7 +163,8 @@ impl NodeConfig {
>>           }
>>       }
>>   
>> -    pub fn set_proxy(&mut self, http_proxy: Option<String>) {
>> +    /// Sets the HTTP proxy configuration
>> +    pub fn set_http_proxy(&mut self, http_proxy: Option<String>) {
>>           self.http_proxy = http_proxy;
>>       }
>>   
>> diff --git a/src/tools/http.rs b/src/tools/http.rs
>> index 998e7647..0821992a 100644
>> --- a/src/tools/http.rs
>> +++ b/src/tools/http.rs
>> @@ -43,7 +43,7 @@ pub(crate) fn build_authority(host: &str, port: u16) -> Result<Authority, Error>
>>   pub struct ProxyConfig {
>>       pub host: String,
>>       pub port: u16,
>> -    pub authorization: Option<String>, // Proxy-Authorization header value
>> +    pub authorization: Option<String>, // user:pass
>>       pub force_connect: bool,
>>   }
>>   
>> @@ -94,7 +94,7 @@ impl ProxyConfig {
>>   
>>               let authority_vec: Vec<&str> = proxy_authority.as_str().rsplitn(2, '@').collect();
>>               let authorization = if authority_vec.len() == 2 {
>> -                Some(format!("Basic {}", base64::encode(authority_vec[1])))
>> +                Some(authority_vec[1].to_string())
> It seems like all changes to this file and config/node.rs could haven been its own patch,
> as they are not directly related to setting the apt proxy, but I'm fine with it as is too.
>
>>               } else {
>>                   None
>>               };
>> @@ -107,6 +107,15 @@ impl ProxyConfig {
>>               })
>>           }).map_err(|err| format_err!("parse_proxy_url failed: {}", err))
>>       }
>> +
>> +    /// Assemble canonical proxy string (including scheme and port)
>> +    pub fn to_proxy_string(&self) -> Result<String, Error> {
>> +        let authority = build_authority(&self.host, self.port)?;
>> +        Ok(match self.authorization {
>> +            None => format!("http://{}", authority),
>> +            Some(ref authorization) => format!("http://{}@{}", authorization, authority)
>> +        })
>> +    }
>>   }
>>   
>>   #[derive(Clone)]
>> @@ -241,7 +250,10 @@ impl hyper::service::Service<Uri> for HttpsConnector {
>>   
>>                       let mut connect_request = format!("CONNECT {0}:{1} HTTP/1.1\r\n", host, port);
>>                       if let Some(authorization) = authorization {
>> -                        connect_request.push_str(&format!("Proxy-Authorization: {}\r\n", authorization));
>> +                        connect_request.push_str(&format!(
>> +                            "Proxy-Authorization: Basic {}\r\n",
>> +                            base64::encode(authorization),
>> +                        ));
>>                       }
>>                       connect_request.push_str(&format!("Host: {0}:{1}\r\n\r\n", host, port));
>>   
>>




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

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

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11  6:54 [pbs-devel] [PATCH proxmox-backup] correctly set apt proxy configuration Dietmar Maurer
2021-05-12 10:01 ` Thomas Lamprecht
2021-05-12 10:21   ` [pbs-devel] applied: " Dietmar Maurer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal