public inbox for pbs-devel@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 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