From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 137271FF15E
for <inbox@lore.proxmox.com>; Tue, 8 Apr 2025 16:06:15 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id C17061F131;
Tue, 8 Apr 2025 16:06:11 +0200 (CEST)
Date: Tue, 8 Apr 2025 16:06:07 +0200
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Christian Ebner <c.ebner@proxmox.com>
Message-ID: <e72lwy5ajpmvq36hqo4egzvbg47i5wu3o7amehtitw7myu3ryv@xvhz2igtj3iw>
References: <20250408123811.178716-1-c.ebner@proxmox.com>
<20250408123811.178716-2-c.ebner@proxmox.com>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <20250408123811.178716-2-c.ebner@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.079 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [connector.rs, simple.rs]
Subject: Re: [pbs-devel] [PATCH v3 proxmox 1/4] http: client: make https
connector generic over resolver
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
One tiny nit...
On Tue, Apr 08, 2025 at 02:38:08PM +0200, Christian Ebner wrote:
> Allow to instantiate a `HttpsConnector` not using the default
> `getaddrinfo` based `GaiResolver` for domain name resolution, but
> rather a custom resolver implementing the required traits.
>
> The usecase for this is to swap out the DNS resolver for the
> statically linked proxmox-backup-client binary, where the glibc
> dependency is problematic because of possible ABI incompatibility.
>
> Adds tower-service as cargo workspace dependency and build dependency
> to debian/control.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 2:
> - no changes
>
> proxmox-http/Cargo.toml | 1 +
> proxmox-http/src/client/connector.rs | 17 ++++++++++++-----
> proxmox-http/src/client/simple.rs | 3 ++-
> 3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/proxmox-http/Cargo.toml b/proxmox-http/Cargo.toml
> index c8c963f7..6bfb1413 100644
> --- a/proxmox-http/Cargo.toml
> +++ b/proxmox-http/Cargo.toml
> @@ -22,6 +22,7 @@ openssl = { version = "0.10", optional = true }
> serde_json = { workspace = true, optional = true }
> tokio = { workspace = true, features = [], optional = true }
> tokio-openssl = { workspace = true, optional = true }
> +tower-service.workspace = true
> ureq = { version = "2.4", features = ["native-certs", "native-tls"], optional = true, default-features = false }
> url = { workspace = true, optional = true }
>
> diff --git a/proxmox-http/src/client/connector.rs b/proxmox-http/src/client/connector.rs
> index 63b9d10c..c0435c60 100644
> --- a/proxmox-http/src/client/connector.rs
> +++ b/proxmox-http/src/client/connector.rs
> @@ -6,6 +6,7 @@ use std::task::{Context, Poll};
>
> use futures::*;
> use http::Uri;
> +use hyper::client::connect::dns::Name;
> use hyper::client::HttpConnector;
> use openssl::ssl::SslConnector;
> use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
> @@ -23,8 +24,8 @@ use crate::{RateLimitedStream, ShareableRateLimit};
> type SharedRateLimit = Arc<dyn ShareableRateLimit>;
>
> #[derive(Clone)]
> -pub struct HttpsConnector {
> - connector: HttpConnector,
> +pub struct HttpsConnector<T> {
Generics on structs can have defaults, so if we use
struct HttpsConnctor<T = GaiResolver> {}
then even the old pbs code builds.
While it is *still* technically a breaking change, other users of this
crate *likely* can do with just a dependency bump.
(But at least it *sort of* documents what the heck this `T` is supposed
to be on a type otherwise completely devoid of documentation ;-) )
> + connector: HttpConnector<T>,
> ssl_connector: Arc<SslConnector>,
> proxy: Option<ProxyConfig>,
> tcp_keepalive: u32,
> @@ -32,9 +33,9 @@ pub struct HttpsConnector {
> write_limiter: Option<SharedRateLimit>,
> }
>
> -impl HttpsConnector {
> +impl<T> HttpsConnector<T> {
> pub fn with_connector(
> - mut connector: HttpConnector,
> + mut connector: HttpConnector<T>,
> ssl_connector: SslConnector,
> tcp_keepalive: u32,
> ) -> Self {
> @@ -122,7 +123,13 @@ impl HttpsConnector {
> }
> }
>
> -impl hyper::service::Service<Uri> for HttpsConnector {
> +impl<T> hyper::service::Service<Uri> for HttpsConnector<T>
> +where
> + T: tower_service::Service<Name> + Clone + Send + Sync + 'static,
> + T::Future: Send,
> + T::Error: Into<Box<(dyn std::error::Error + Send + Sync + 'static)>>,
> + T::Response: std::iter::Iterator<Item = std::net::SocketAddr>,
> +{
> type Response = MaybeTlsStream<RateLimitedStream<TcpStream>>;
> type Error = Error;
> #[allow(clippy::type_complexity)]
> diff --git a/proxmox-http/src/client/simple.rs b/proxmox-http/src/client/simple.rs
> index 062889ac..cb8bb777 100644
> --- a/proxmox-http/src/client/simple.rs
> +++ b/proxmox-http/src/client/simple.rs
> @@ -8,6 +8,7 @@ use futures::*;
> #[cfg(all(feature = "client-trait", feature = "proxmox-async"))]
> use http::header::HeaderName;
> use http::{HeaderValue, Request, Response};
> +use hyper::client::connect::dns::GaiResolver;
> use hyper::client::Client as HyperClient;
> use hyper::client::HttpConnector;
> use hyper::Body;
> @@ -18,7 +19,7 @@ use crate::HttpOptions;
>
> /// Asynchronous HTTP client implementation
> pub struct Client {
> - client: HyperClient<HttpsConnector, Body>,
> + client: HyperClient<HttpsConnector<GaiResolver>, Body>,
> options: HttpOptions,
> }
>
> --
> 2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel