From 42970526bdbca2363d52b1f5fdfe46db1fced08d Mon Sep 17 00:00:00 2001 From: omavashia Date: Thu, 25 Jun 2026 15:19:51 -0700 Subject: [PATCH 1/9] implemented TODO (wrapped Acceptor in Result so thread does not panic) --- pingora-cache/src/cache_control.rs | 2 +- pingora-core/src/listeners/mod.rs | 2 +- pingora-core/src/listeners/tls/rustls/mod.rs | 7 +++---- pingora-core/src/protocols/tls/noop_tls/mod.rs | 4 ++-- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pingora-cache/src/cache_control.rs b/pingora-cache/src/cache_control.rs index f8612080..7ffeafc9 100644 --- a/pingora-cache/src/cache_control.rs +++ b/pingora-cache/src/cache_control.rs @@ -1,4 +1,4 @@ -// Copyright 2026 Cloudflare, Inc. +/Copyright 2026 Cloudflare, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/pingora-core/src/listeners/mod.rs b/pingora-core/src/listeners/mod.rs index db799765..392f4cc0 100644 --- a/pingora-core/src/listeners/mod.rs +++ b/pingora-core/src/listeners/mod.rs @@ -190,7 +190,7 @@ impl TransportStackBuilder { Ok(TransportStack { l4, - tls: self.tls.take().map(|tls| Arc::new(tls.build())), + tls: self.tls.take().map(|tls| Arc::new(tls.build().unwrap())), l4_buffer: self.l4_buffer, pre_tls_callback: self.pre_tls_callback.clone(), }) diff --git a/pingora-core/src/listeners/tls/rustls/mod.rs b/pingora-core/src/listeners/tls/rustls/mod.rs index e7376fc0..f0779274 100644 --- a/pingora-core/src/listeners/tls/rustls/mod.rs +++ b/pingora-core/src/listeners/tls/rustls/mod.rs @@ -46,8 +46,7 @@ impl TlsSettings { /// _NOTE_ This function will panic if there is an error in loading /// certificate files or constructing the builder /// - /// Todo: Return a result instead of panicking XD - pub fn build(self) -> Acceptor { + pub fn build(self) -> Result { // rustls 0.23+ requires an explicit CryptoProvider. pingora_rustls::install_default_crypto_provider(); @@ -77,10 +76,10 @@ impl TlsSettings { config.alpn_protocols = alpn_protocols; } - Acceptor { + Ok(Acceptor { acceptor: RusTlsAcceptor::from(Arc::new(config)), callbacks: None, - } + }) } /// Enable HTTP/2 support for this endpoint, which is default off. diff --git a/pingora-core/src/protocols/tls/noop_tls/mod.rs b/pingora-core/src/protocols/tls/noop_tls/mod.rs index d7632e13..b24a5b35 100644 --- a/pingora-core/src/protocols/tls/noop_tls/mod.rs +++ b/pingora-core/src/protocols/tls/noop_tls/mod.rs @@ -80,8 +80,8 @@ pub mod listeners { pub struct TlsSettings; impl TlsSettings { - pub fn build(&self) -> Acceptor { - Acceptor + pub fn build(&self) -> Result { + Ok(Acceptor) } pub fn intermediate(_: &str, _: &str) -> Result { From ef11cb2b8b1873e625852ba7ecf21c7958e5c9a8 Mon Sep 17 00:00:00 2001 From: omavashia Date: Thu, 25 Jun 2026 16:27:15 -0700 Subject: [PATCH 2/9] made error propagation more apparent - removed unwrap from calls to tls.build() --- pingora-core/src/listeners/mod.rs | 7 ++++++- .../src/listeners/tls/boringssl_openssl/mod.rs | 13 ++++++++----- pingora-core/src/listeners/tls/s2n/mod.rs | 10 ++++++---- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/pingora-core/src/listeners/mod.rs b/pingora-core/src/listeners/mod.rs index 392f4cc0..4fb13c83 100644 --- a/pingora-core/src/listeners/mod.rs +++ b/pingora-core/src/listeners/mod.rs @@ -188,9 +188,14 @@ impl TransportStackBuilder { #[cfg(windows)] let l4 = builder.listen().await?; + let tls_val = match self.tls.take(){ + Some(tls) => Some(Arc::new(tls.build()?)), + None => None, + }; + Ok(TransportStack { l4, - tls: self.tls.take().map(|tls| Arc::new(tls.build().unwrap())), + tls: tls_val, l4_buffer: self.l4_buffer, pre_tls_callback: self.pre_tls_callback.clone(), }) diff --git a/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs b/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs index d957cac4..11e2c1b4 100644 --- a/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs +++ b/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs @@ -129,11 +129,14 @@ impl TlsSettings { } } - pub(crate) fn build(self) -> Acceptor { - Acceptor { - ssl_acceptor: self.accept_builder.build(), - callbacks: self.callbacks, - } + pub(crate) fn build(self) -> Result { + Ok( + Acceptor { + ssl_acceptor: self.accept_builder.build(), + callbacks: self.callbacks, + } + + ) } } diff --git a/pingora-core/src/listeners/tls/s2n/mod.rs b/pingora-core/src/listeners/tls/s2n/mod.rs index af547bbe..aa696b40 100644 --- a/pingora-core/src/listeners/tls/s2n/mod.rs +++ b/pingora-core/src/listeners/tls/s2n/mod.rs @@ -43,7 +43,7 @@ pub struct Acceptor { } impl TlsSettings { - pub fn build(self) -> Acceptor { + pub fn build(self) -> Result { let mut builder = Config::builder(); // Default security policy with TLS 1.3 support @@ -94,9 +94,11 @@ impl TlsSettings { security_policy: Some(policy.clone()), }; - Acceptor { - acceptor: TlsAcceptor::new(connection_builder), - } + Ok( + Acceptor { + acceptor: TlsAcceptor::new(connection_builder), + } + ) } /// Enable HTTP/2 support for this endpoint, which is default off. From dd6ccb819f1a812ad246fde22aeb767c56c85b89 Mon Sep 17 00:00:00 2001 From: omavashia Date: Thu, 25 Jun 2026 16:38:55 -0700 Subject: [PATCH 3/9] running cargo fmt --- pingora-cache/src/cache_control.rs | 5 ++--- pingora-core/src/listeners/mod.rs | 2 +- .../src/listeners/tls/boringssl_openssl/mod.rs | 11 ++++------- pingora-core/src/listeners/tls/s2n/mod.rs | 8 +++----- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/pingora-cache/src/cache_control.rs b/pingora-cache/src/cache_control.rs index 7ffeafc9..904e5cf9 100644 --- a/pingora-cache/src/cache_control.rs +++ b/pingora-cache/src/cache_control.rs @@ -1,4 +1,4 @@ -/Copyright 2026 Cloudflare, Inc. +///Copyright 2026 Cloudflare, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Functions and utilities to help parse Cache-Control headers - +/// Functions and utilities to help parse Cache-Control headers use super::*; use http::header::HeaderName; diff --git a/pingora-core/src/listeners/mod.rs b/pingora-core/src/listeners/mod.rs index 4fb13c83..ebcc9471 100644 --- a/pingora-core/src/listeners/mod.rs +++ b/pingora-core/src/listeners/mod.rs @@ -188,7 +188,7 @@ impl TransportStackBuilder { #[cfg(windows)] let l4 = builder.listen().await?; - let tls_val = match self.tls.take(){ + let tls_val = match self.tls.take() { Some(tls) => Some(Arc::new(tls.build()?)), None => None, }; diff --git a/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs b/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs index 11e2c1b4..17ed8b94 100644 --- a/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs +++ b/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs @@ -130,13 +130,10 @@ impl TlsSettings { } pub(crate) fn build(self) -> Result { - Ok( - Acceptor { - ssl_acceptor: self.accept_builder.build(), - callbacks: self.callbacks, - } - - ) + Ok(Acceptor { + ssl_acceptor: self.accept_builder.build(), + callbacks: self.callbacks, + }) } } diff --git a/pingora-core/src/listeners/tls/s2n/mod.rs b/pingora-core/src/listeners/tls/s2n/mod.rs index aa696b40..72c0bd9d 100644 --- a/pingora-core/src/listeners/tls/s2n/mod.rs +++ b/pingora-core/src/listeners/tls/s2n/mod.rs @@ -94,11 +94,9 @@ impl TlsSettings { security_policy: Some(policy.clone()), }; - Ok( - Acceptor { - acceptor: TlsAcceptor::new(connection_builder), - } - ) + Ok(Acceptor { + acceptor: TlsAcceptor::new(connection_builder), + }) } /// Enable HTTP/2 support for this endpoint, which is default off. From 643b5cfc3b6ca025e3ad5f0e31be3ed7300fe619 Mon Sep 17 00:00:00 2001 From: omavashia Date: Thu, 25 Jun 2026 16:58:22 -0700 Subject: [PATCH 4/9] attempting to fix cargo clippy error --- pingora-cache/src/cache_control.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pingora-cache/src/cache_control.rs b/pingora-cache/src/cache_control.rs index 904e5cf9..94b9ff45 100644 --- a/pingora-cache/src/cache_control.rs +++ b/pingora-cache/src/cache_control.rs @@ -1,4 +1,4 @@ -///Copyright 2026 Cloudflare, Inc. +// Copyright 2026 Cloudflare, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From c210840643e7b4d3b638c90c8cc5b72611b5c0ba Mon Sep 17 00:00:00 2001 From: omavashia Date: Fri, 26 Jun 2026 11:27:12 -0700 Subject: [PATCH 5/9] reverted comment change --- pingora-cache/src/cache_control.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pingora-cache/src/cache_control.rs b/pingora-cache/src/cache_control.rs index 94b9ff45..584d8b29 100644 --- a/pingora-cache/src/cache_control.rs +++ b/pingora-cache/src/cache_control.rs @@ -1,4 +1,4 @@ -// Copyright 2026 Cloudflare, Inc. +//! Copyright 2026 Cloudflare, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 4451b07619860465577749be15d07472164ccffe Mon Sep 17 00:00:00 2001 From: omavashia Date: Fri, 26 Jun 2026 11:44:44 -0700 Subject: [PATCH 6/9] converted panic to err in rustls build function if certificate fails to load. --- pingora-core/src/listeners/tls/rustls/mod.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pingora-core/src/listeners/tls/rustls/mod.rs b/pingora-core/src/listeners/tls/rustls/mod.rs index f0779274..01478b9d 100644 --- a/pingora-core/src/listeners/tls/rustls/mod.rs +++ b/pingora-core/src/listeners/tls/rustls/mod.rs @@ -52,10 +52,13 @@ impl TlsSettings { let Ok(Some((certs, key))) = load_certs_and_key_files(&self.cert_path, &self.key_path) else { - panic!( - "Failed to load provided certificates \"{}\" or key \"{}\".", - self.cert_path, self.key_path - ) + return Error::e_explain( + ErrorType::InternalError, + format!( + "Failed to load provided certificates \"{}\" or key \"{}\".", + self.cert_path, self.key_path, + ), + ); }; let builder = From e0243210903dcb13027ac4d9274b5b31b8a48f1b Mon Sep 17 00:00:00 2001 From: omavashia Date: Fri, 26 Jun 2026 12:46:20 -0700 Subject: [PATCH 7/9] replacing panic with error --- pingora-core/src/listeners/tls/s2n/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pingora-core/src/listeners/tls/s2n/mod.rs b/pingora-core/src/listeners/tls/s2n/mod.rs index 72c0bd9d..a35f0cff 100644 --- a/pingora-core/src/listeners/tls/s2n/mod.rs +++ b/pingora-core/src/listeners/tls/s2n/mod.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use log::debug; -use pingora_error::Result; +use pingora_error::{Error, OrErr, Result}; use pingora_s2n::{ load_certs_and_key_files, ClientAuthType, Config, IgnoreVerifyHostnameCallback, S2NPolicy, TlsAcceptor, DEFAULT_TLS13, @@ -82,9 +82,9 @@ impl TlsSettings { } if !self.verify_client_hostname { - builder - .set_verify_host_callback(IgnoreVerifyHostnameCallback::new()) - .unwrap(); + if let Err(_) = builder.set_verify_host_callback(IgnoreVerifyHostnameCallback::new()) { + return Err(ErrorType::InternalError, "Failed to verify client hostname"); + }; } let config = builder.build().unwrap(); From 91e9d312a150bf773f828679f250bc63bdcb3fbc Mon Sep 17 00:00:00 2001 From: omavashia Date: Fri, 26 Jun 2026 13:30:09 -0700 Subject: [PATCH 8/9] attempting to fix cargo build errors for s2n and rustls --- pingora-core/src/listeners/tls/rustls/mod.rs | 18 +++++++++++------- pingora-core/src/listeners/tls/s2n/mod.rs | 12 +++++++++--- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/pingora-core/src/listeners/tls/rustls/mod.rs b/pingora-core/src/listeners/tls/rustls/mod.rs index 01478b9d..90b9dda8 100644 --- a/pingora-core/src/listeners/tls/rustls/mod.rs +++ b/pingora-core/src/listeners/tls/rustls/mod.rs @@ -18,7 +18,7 @@ use crate::listeners::TlsAcceptCallbacks; use crate::protocols::tls::{server::handshake, server::handshake_with_callback, TlsStream}; use log::debug; use pingora_error::ErrorType::InternalError; -use pingora_error::{Error, OrErr, Result}; +use pingora_error::{Error, OrErr, Result, ErrorType, ErrorSource, RetryType, ImmutStr}; use pingora_rustls::load_certs_and_key_files; use pingora_rustls::ClientCertVerifier; use pingora_rustls::ServerConfig; @@ -52,13 +52,17 @@ impl TlsSettings { let Ok(Some((certs, key))) = load_certs_and_key_files(&self.cert_path, &self.key_path) else { - return Error::e_explain( - ErrorType::InternalError, - format!( + let error_message = format!( "Failed to load provided certificates \"{}\" or key \"{}\".", - self.cert_path, self.key_path, - ), - ); + self.cert_path, self.key_path); + + return Err(Box::new(Error { + etype: ErrorType::InternalError, + esource: ErrorSource::Internal, + retry: RetryType::Decided(false), + cause: None, + context: Some(ImmutStr::Owned(error_message.into_boxed_str())) + })); }; let builder = diff --git a/pingora-core/src/listeners/tls/s2n/mod.rs b/pingora-core/src/listeners/tls/s2n/mod.rs index a35f0cff..d6da847e 100644 --- a/pingora-core/src/listeners/tls/s2n/mod.rs +++ b/pingora-core/src/listeners/tls/s2n/mod.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use log::debug; -use pingora_error::{Error, OrErr, Result}; +use pingora_error::{Result, ErrorType, Error, ErrorSource, RetryType, ImmutStr}; use pingora_s2n::{ load_certs_and_key_files, ClientAuthType, Config, IgnoreVerifyHostnameCallback, S2NPolicy, TlsAcceptor, DEFAULT_TLS13, @@ -83,8 +83,14 @@ impl TlsSettings { if !self.verify_client_hostname { if let Err(_) = builder.set_verify_host_callback(IgnoreVerifyHostnameCallback::new()) { - return Err(ErrorType::InternalError, "Failed to verify client hostname"); - }; + return Err(Box::new(Error { + etype: ErrorType::InternalError, + esource: ErrorSource::Internal, + retry: RetryType::Decided(false), + cause: None, + context: Some(ImmutStr::from("Failed to verify client hostname")), + })); + } } let config = builder.build().unwrap(); From a37a46f08ca7f611bddd52a3a5e0d484ed71eec4 Mon Sep 17 00:00:00 2001 From: omavashia Date: Fri, 26 Jun 2026 13:30:51 -0700 Subject: [PATCH 9/9] running cargo fmt --- pingora-core/src/listeners/tls/rustls/mod.rs | 13 +++++++------ pingora-core/src/listeners/tls/s2n/mod.rs | 6 +++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/pingora-core/src/listeners/tls/rustls/mod.rs b/pingora-core/src/listeners/tls/rustls/mod.rs index 90b9dda8..e1fa52c8 100644 --- a/pingora-core/src/listeners/tls/rustls/mod.rs +++ b/pingora-core/src/listeners/tls/rustls/mod.rs @@ -18,7 +18,7 @@ use crate::listeners::TlsAcceptCallbacks; use crate::protocols::tls::{server::handshake, server::handshake_with_callback, TlsStream}; use log::debug; use pingora_error::ErrorType::InternalError; -use pingora_error::{Error, OrErr, Result, ErrorType, ErrorSource, RetryType, ImmutStr}; +use pingora_error::{Error, ErrorSource, ErrorType, ImmutStr, OrErr, Result, RetryType}; use pingora_rustls::load_certs_and_key_files; use pingora_rustls::ClientCertVerifier; use pingora_rustls::ServerConfig; @@ -53,15 +53,16 @@ impl TlsSettings { let Ok(Some((certs, key))) = load_certs_and_key_files(&self.cert_path, &self.key_path) else { let error_message = format!( - "Failed to load provided certificates \"{}\" or key \"{}\".", - self.cert_path, self.key_path); + "Failed to load provided certificates \"{}\" or key \"{}\".", + self.cert_path, self.key_path + ); return Err(Box::new(Error { etype: ErrorType::InternalError, - esource: ErrorSource::Internal, - retry: RetryType::Decided(false), + esource: ErrorSource::Internal, + retry: RetryType::Decided(false), cause: None, - context: Some(ImmutStr::Owned(error_message.into_boxed_str())) + context: Some(ImmutStr::Owned(error_message.into_boxed_str())), })); }; diff --git a/pingora-core/src/listeners/tls/s2n/mod.rs b/pingora-core/src/listeners/tls/s2n/mod.rs index d6da847e..e9cfc091 100644 --- a/pingora-core/src/listeners/tls/s2n/mod.rs +++ b/pingora-core/src/listeners/tls/s2n/mod.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use log::debug; -use pingora_error::{Result, ErrorType, Error, ErrorSource, RetryType, ImmutStr}; +use pingora_error::{Error, ErrorSource, ErrorType, ImmutStr, Result, RetryType}; use pingora_s2n::{ load_certs_and_key_files, ClientAuthType, Config, IgnoreVerifyHostnameCallback, S2NPolicy, TlsAcceptor, DEFAULT_TLS13, @@ -85,8 +85,8 @@ impl TlsSettings { if let Err(_) = builder.set_verify_host_callback(IgnoreVerifyHostnameCallback::new()) { return Err(Box::new(Error { etype: ErrorType::InternalError, - esource: ErrorSource::Internal, - retry: RetryType::Decided(false), + esource: ErrorSource::Internal, + retry: RetryType::Decided(false), cause: None, context: Some(ImmutStr::from("Failed to verify client hostname")), }));