Skip to content
Closed
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
13e0cfd
update test_https to use local http server
ton-anywhere Feb 9, 2026
e879020
add test_https_with_client
ton-anywhere Feb 9, 2026
182e5fb
WIP: add ClientBuilder for configuring Client instances
ton-anywhere Feb 11, 2026
92ca975
WIP: pass ClientConfig struct to tls layer
ton-anywhere Feb 12, 2026
5af7539
WIP: include feature on ClientConfig import
ton-anywhere Feb 12, 2026
b6650ba
WIP append custom cert
ton-anywhere Feb 12, 2026
bf1735d
WIP: update tests
ton-anywhere Feb 12, 2026
da22cf9
rename TlsConfig cert attribute
ton-anywhere Feb 12, 2026
a01979a
remove comment
ton-anywhere Feb 12, 2026
d89f559
style adjustment
ton-anywhere Feb 12, 2026
04a1c3a
add example
ton-anywhere Feb 12, 2026
7011045
Code review adjustment: Use AsyncConnection::new instead of new_with_…
ton-anywhere Feb 13, 2026
deb5397
WIP: include certificates on TlsConfig struct
ton-anywhere Feb 13, 2026
5a97e80
style adjustment
ton-anywhere Feb 13, 2026
a61b1ec
make rustls_stream mod public temporarily
ton-anywhere Feb 13, 2026
8000c6c
WIP: create Certificates wrapper on rustls_stream mod
ton-anywhere Feb 13, 2026
9b4a839
WIP use custom error when appending a certificate
ton-anywhere Feb 13, 2026
8d7ff6c
WIP remove moved code
ton-anywhere Feb 13, 2026
0538906
WIP remove unused field from TlsConfig
ton-anywhere Feb 13, 2026
5fc031b
add Certificates module
ton-anywhere Feb 13, 2026
9c11211
adjust privacy on structs
ton-anywhere Feb 13, 2026
22c2b2f
add new docs
ton-anywhere Feb 13, 2026
97b5d56
update doc and example
ton-anywhere Feb 13, 2026
4a08c7d
remove comment
ton-anywhere Feb 13, 2026
2cf40aa
Adjust custom_cert example feature
ton-anywhere Feb 16, 2026
937e3ba
fix: correct feature flag for CustomClientConfig import
ton-anywhere Feb 16, 2026
e682f92
List adjustments
ton-anywhere Feb 16, 2026
fd47ea1
fix Cargo fmt adjustments
ton-anywhere Feb 16, 2026
728ceb4
Rename `certificate` to `cert_der`
ton-anywhere Feb 16, 2026
c9d941d
take ownership of cert_der on append_certificate
ton-anywhere Feb 16, 2026
0f67697
rename parameter cert_der on Doc for with_root_certificate
ton-anywhere Feb 16, 2026
65e7c41
Reuse existing TLSConfig if possible - allows for multiple certificat…
ton-anywhere Feb 16, 2026
ecf4d12
Update TlsConfig::new and ClientBuilder::with_root_certificate to ret…
ton-anywhere Feb 17, 2026
c11c920
Update custom_cert example with new return from with_root_certificate…
ton-anywhere Feb 18, 2026
cc84c5c
Fix test flags
ton-anywhere Feb 19, 2026
abf89cf
warnings fix
ton-anywhere Feb 19, 2026
e8f47b6
fix: unresolved import - Refactor client mod to allow cleaner conditi…
ton-anywhere Feb 19, 2026
97124a9
Gate tls modules declarations
ton-anywhere Feb 19, 2026
8382cc1
Fix doctest
ton-anywhere Feb 19, 2026
05f8e5d
remove connector caching with client_config - always create new conne…
ton-anywhere Feb 21, 2026
2ef687e
Improve code organization: Move Client and ClientImpl declarations be…
ton-anywhere Feb 22, 2026
bfe2763
wrap ClientConfig with Arc smart pointer to reduce memory usage
ton-anywhere Feb 22, 2026
b2fe156
Merge branch 'master' into bitreq-client-builder
ton-anywhere Feb 22, 2026
f3851fb
use Arc clone instead of option clone
ton-anywhere Feb 22, 2026
6ac03d7
Wrap Certificates with arc and inject from Client - load root_certs o…
ton-anywhere Feb 23, 2026
bf3a952
bump default Client default connection pool(capacity) to 10
ton-anywhere Feb 23, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions bitreq/examples/custom_cert.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//! This example demonstrates the client builder with custom DER certificate.
//! to run: cargo run --example custom_cert --features async-https-rustls

#[cfg(not(feature = "async-https-rustls"))]
fn main() {
println!("This example requires the 'async-https-rustls' feature.");
}

#[cfg(feature = "async-https-rustls")]
fn main() -> Result<(), bitreq::Error> {
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_io()
.build()
.expect("failed to build Tokio runtime");

runtime.block_on(request_with_client())
}

#[cfg(feature = "async-https-rustls")]
async fn request_with_client() -> Result<(), bitreq::Error> {
let url = "http://example.com";
let cert_der = include_bytes!("../tests/test_cert.der");
let client = bitreq::Client::builder().with_root_certificate(cert_der.as_slice()).build();
// OR
// let cert_der: &[u8] = include_bytes!("../tests/test_cert.der");
// let client = bitreq::Client::builder().with_root_certificate(cert_der).build();
// OR
// let cert_vec: Vec<u8> = include_bytes!("../tests/test_cert.der").to_vec();
// let client = bitreq::Client::builder().with_root_certificate(cert_vec.as_slice()).build();

let response = client.send_async(bitreq::get(url)).await.unwrap();

println!("Status: {}", response.status_code);
println!("Body: {}", response.as_str()?);

Ok(())
}
147 changes: 145 additions & 2 deletions bitreq/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use std::collections::{hash_map, HashMap, VecDeque};
use std::sync::{Arc, Mutex};

use crate::connection::certificates::Certificates;
use crate::connection::AsyncConnection;
use crate::request::{OwnedConnectionParams as ConnectionKey, ParsedRequest};
use crate::{Error, Request, Response};
Expand Down Expand Up @@ -39,10 +40,142 @@ struct ClientImpl<T> {
connections: HashMap<ConnectionKey, Arc<T>>,
lru_order: VecDeque<ConnectionKey>,
capacity: usize,
client_config: Option<ClientConfig>,
}

pub struct ClientBuilder {
capacity: usize,
client_config: Option<ClientConfig>,
}

#[derive(Clone)]
pub(crate) struct ClientConfig {
pub(crate) tls: Option<TlsConfig>,
}

#[derive(Clone)]
pub(crate) struct TlsConfig {
pub(crate) certificates: Certificates,
}

impl TlsConfig {
fn new(certificate: Vec<u8>) -> Self {
let certificates =
Certificates::new(Some(&certificate)).expect("failed to append certificate");

Self { certificates }
}
}

/// Builder for configuring a `Client` with custom settings.
///
/// The builder allows you to set the connection pool capacity and add
/// custom root certificates for TLS verification before constructing the client.
///
/// # Example
///
/// ```no_run
/// # async fn example() -> Result<(), bitreq::Error> {
/// use bitreq::{Client, RequestExt};
///
/// let cert_der = include_bytes!("../tests/test_cert.der");
/// let client = Client::builder()
/// .with_root_certificate(cert_der.as_slice())
/// .with_capacity(20)
/// .build();
///
/// let response = bitreq::get("https://example.com")
/// .send_async_with_client(&client)
/// .await?;
/// # Ok(())
/// # }
/// ```
impl ClientBuilder {
/// Creates a new `ClientBuilder` with default settings.
///
/// Default configuration:
/// * `capacity` - 1 (single connection)
/// * `root_certificates` - None (uses system certificates)
pub fn new() -> Self { Self { capacity: 1, client_config: None } }

/// Adds a custom root certificate for TLS verification.
///
/// The certificate must be provided in DER format. This method accepts any type
/// that can be converted into a `Vec<u8>`, such as `Vec<u8>`, `&[u8]`, or arrays.
/// This is useful when connecting to servers using self-signed certificates
/// or custom Certificate Authorities.
///
/// # Arguments
///
/// * `certificate` - A DER-encoded X.509 certificate. Accepts any type that implements
/// `Into<Vec<u8>>` (e.g., `&[u8]`, `Vec<u8>`, or `[u8; N]`).
///
/// # Example
///
/// ```no_run
/// # use bitreq::Client;
/// // Using a byte slice
/// let cert_der: &[u8] = include_bytes!("../tests/test_cert.der");
/// let client = Client::builder()
/// .with_root_certificate(cert_der)
/// .build();
///
/// // Using a Vec<u8>
/// let cert_vec: Vec<u8> = cert_der.to_vec();
/// let client = Client::builder()
/// .with_root_certificate(cert_vec)
/// .build();
/// ```
pub fn with_root_certificate<T: Into<Vec<u8>>>(mut self, certificate: T) -> Self {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its pretty awkward that we don't error here but then just ignore certs silently. IMO we kinda need to actually make this fallible, store a root cert store in ClientConfig, and pass that around instead. We'll have to have an abstract root cert store in rustls_stream (or somewhere), but that should be pretty straightforward.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheBlueMatt makes sense. Will work on that tomorrow! Thanks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheBlueMatt updated the PR with your suggestion. I moved the new certificates logic to a new module. We still have lots of stuff in the rustls_stream module, but I thought refactoring that would make this PR too large.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still looks like its currently infallible and not parsing the cert? We should validate the cert here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @TheBlueMatt. Currently the method is fallible, it panics on expect() if certificate appending fails. However, I intentionally kept the return type as Self (not Result<Self, Error>) to maintain fluent builder chaining without requiring callers to handle Result at every step. Would you prefer that I return Result<Self, Error> here?

About parsing, I don't know if I understand you're question well, I'm sorry. But we're currently parsing it on the new Certificates module. I tried to keep this Client interface simple and more details about certificate management inside this module.

https://github.com/rust-bitcoin/corepc/pull/502/changes#diff-4501e006f554583aacf6f0039d2c12e149a90447ec1af22f3c68d74820932c5fR28

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, panicing isn't really "fallible" in Rust lingo :). Yes, we should return a Result.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the return type @TheBlueMatt. Let me know about the cert parsing you've mentioned.
With the updated code we're receiving encoding errors as well.

Print from a test I've run with an invalid certificate
image

let tls_config = TlsConfig::new(certificate.into());
self.client_config = Some(ClientConfig { tls: Some(tls_config) });
Comment thread
ton-anywhere marked this conversation as resolved.
Outdated
self
}

/// Sets the maximum number of connections to keep in the pool.
///
/// When the pool reaches this capacity, the least recently used connection
/// is evicted to make room for new connections.
///
/// # Arguments
///
/// * `capacity` - Maximum number of cached connections
///
/// # Example
///
/// ```no_run
/// # use bitreq::Client;
/// let client = Client::builder()
/// .with_capacity(10)
/// .build();
/// ```
pub fn with_capacity(mut self, capacity: usize) -> Self {
self.capacity = capacity;
self
}

/// Builds the `Client` with the configured settings.
///
/// Consumes the builder and returns a configured `Client` instance
/// ready to send requests with connection pooling.
pub fn build(self) -> Client {
Client {
r#async: Arc::new(Mutex::new(ClientImpl {
connections: HashMap::new(),
lru_order: VecDeque::new(),
capacity: self.capacity,
client_config: self.client_config,
})),
}
}
}

impl Default for ClientBuilder {
fn default() -> Self { Self::new() }
}

impl Client {
/// Creates a new `Client` with the specified connection cache capacity.
/// Creates a new `Client` with the specified connection pool capacity.
///
/// # Arguments
///
Expand All @@ -54,10 +187,14 @@ impl Client {
connections: HashMap::new(),
lru_order: VecDeque::new(),
capacity,
client_config: None,
})),
}
}

/// Create a builder for a client
pub fn builder() -> ClientBuilder { ClientBuilder::new() }

/// Sends a request asynchronously using a cached connection if available.
pub async fn send_async(&self, request: Request) -> Result<Response, Error> {
let parsed_request = ParsedRequest::new(request)?;
Expand All @@ -77,7 +214,13 @@ impl Client {
let conn = if let Some(conn) = conn_opt {
conn
} else {
let connection = AsyncConnection::new(key, parsed_request.timeout_at).await?;
let client_config = {
let state = self.r#async.lock().unwrap();
state.client_config.clone()
};

let connection =
AsyncConnection::new(key, parsed_request.timeout_at, client_config).await?;
let connection = Arc::new(connection);

let mut state = self.r#async.lock().unwrap();
Expand Down
39 changes: 33 additions & 6 deletions bitreq/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,17 @@ use tokio::net::TcpStream as AsyncTcpStream;
#[cfg(feature = "async")]
use tokio::sync::Mutex as AsyncMutex;

#[cfg(feature = "async")]
use crate::client::ClientConfig;
use crate::request::{ConnectionParams, OwnedConnectionParams, ParsedRequest};
#[cfg(feature = "async")]
use crate::Response;
use crate::{Error, Method, ResponseLazy};

type UnsecuredStream = TcpStream;

#[cfg(feature = "rustls")]
pub(crate) mod certificates;
#[cfg(feature = "rustls")]
mod rustls_stream;
#[cfg(feature = "rustls")]
Expand Down Expand Up @@ -266,15 +270,13 @@ impl AsyncConnection {
pub(crate) async fn new(
params: ConnectionParams<'_>,
timeout_at: Option<Instant>,
client_config: Option<ClientConfig>,
) -> Result<AsyncConnection, Error> {
let future = async move {
let socket = Self::connect(params).await?;

if params.https {
#[cfg(not(feature = "tokio-rustls"))]
return Err(Error::HttpsFeatureNotEnabled);
#[cfg(feature = "tokio-rustls")]
rustls_stream::wrap_async_stream(socket, params.host).await
Self::wrap_async_stream(socket, params.host, client_config).await
} else {
Ok(AsyncHttpStream::Unsecured(socket))
}
Expand All @@ -298,6 +300,30 @@ impl AsyncConnection {
}))))
}

/// Call the correct wrapper function depending on whether client_configs are present
#[cfg(all(feature = "rustls", feature = "tokio-rustls"))]
async fn wrap_async_stream(
socket: AsyncTcpStream,
host: &str,
client_config: Option<ClientConfig>,
) -> Result<AsyncHttpStream, Error> {
if let Some(client_config) = client_config {
rustls_stream::wrap_async_stream_with_configs(socket, host, client_config).await
} else {
rustls_stream::wrap_async_stream(socket, host).await
}
}

/// Error treatment function, should not be called under normal circustances
#[cfg(not(all(feature = "rustls", feature = "tokio-rustls")))]
async fn wrap_async_stream(
_socket: AsyncTcpStream,
_host: &str,
_client_config: Option<ClientConfig>,
) -> Result<AsyncHttpStream, Error> {
Err(Error::HttpsFeatureNotEnabled)
}

async fn tcp_connect(host: &str, port: u16) -> Result<AsyncTcpStream, Error> {
#[cfg(feature = "log")]
log::trace!("Looking up host {host}");
Expand Down Expand Up @@ -447,7 +473,7 @@ impl AsyncConnection {
};
(_internal) => {
let new_connection =
AsyncConnection::new(request.connection_params(), request.timeout_at)
AsyncConnection::new(request.connection_params(), request.timeout_at, None)
.await?;
*self.0.lock().unwrap() = Arc::clone(&*new_connection.0.lock().unwrap());
core::mem::drop(read);
Expand Down Expand Up @@ -806,7 +832,8 @@ async fn async_handle_redirects(
let new_connection;
if needs_new_connection {
new_connection =
AsyncConnection::new(request.connection_params(), request.timeout_at).await?;
AsyncConnection::new(request.connection_params(), request.timeout_at, None)
.await?;
connection = &new_connection;
}
connection.send(request).await
Expand Down
63 changes: 63 additions & 0 deletions bitreq/src/connection/certificates.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#[cfg(feature = "rustls")]
use rustls::RootCertStore;
#[cfg(feature = "rustls-webpki")]
use webpki_roots::TLS_SERVER_ROOTS;

use crate::Error;

#[derive(Clone)]
pub(crate) struct Certificates {
pub(crate) inner: RootCertStore,
}

impl Certificates {
pub(crate) fn new(certificate: Option<&Vec<u8>>) -> Result<Self, Error> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is just a Vec<u8> tell me what format you're expecting input both in the param name and docstring

Suggested change
pub(crate) fn new(certificate: Option<&Vec<u8>>) -> Result<Self, Error> {
/// Pass a new certificate in DER [?] format
pub(crate) fn new(certificate_der: Option<&Vec<u8>>) -> Result<Self, Error> {

Copy link
Copy Markdown
Contributor Author

@ton-anywhere ton-anywhere Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanGould Great catch. Yes, you're correct. We must receive a DER encoded certificate.
I've updated the variable to cert_der to give more clarity. About the doc, I've written on the Client class since it's public.

https://github.com/rust-bitcoin/corepc/pull/502/changes#diff-21f18a8053f4d7bce98cbe0c84adc7e943caf49c0a245be26096be8ce461ae64R53-R82

What do you think?

let certificates = Self { inner: RootCertStore::empty() };

if let Some(certificate) = certificate {
certificates.append_certificate(certificate)
} else {
Ok(certificates)
}
}

#[cfg(feature = "rustls")]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need support for native-tls, too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheBlueMatt heads up - we currently don't support native-tls for async
connections. The AsyncConnection only calls wrap_async_stream for the
tokio-rustls feature:

https://github.com/rust-bitcoin/corepc/blob/master/bitreq/src/connection.rs#L275

Interestingly, wrap_async_stream exists for tokio-native-tls too but isn't
being called from AsyncConnection:

https://github.com/rust-bitcoin/corepc/blob/master/bitreq/src/connection/rustls_stream.rs#L150

Would you prefer to:

  1. Add native-tls support in this PR by hooking up the existing
    wrap_async_stream for tokio-native-tls?
  2. Defer it to a follow-up PR?

Happy to implement either way.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? Its definitely called for async connection, just only in some feature flag combos.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheBlueMatt I've been working on the native-tls implementation, but the changes ended up being quite large. Adding them on top of this PR would (in my opinion) make it significantly harder to review and test properly.

That's why I created a separate draft PR here to continue that work:
ton-anywhere#1

In my view, this PR already delivers value for users relying on rustls.

What do you think about keeping this one strictly focused on rustls (and handling native-tls in the other PR)?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we should definitely do both. We shouldn't land things that leave the codebase in a broken state, and that PR is not particularly large. There's a lot of random code to review in this PR as is that handles the no-support case which I'd rather not do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheBlueMatt No problem then, will merge both features. New PR or we keep in this one?

I don't know if I understand your point about broken state, do you mean having it implemented partially by features or do you see something that could cause a regression?

Copy link
Copy Markdown
Contributor Author

@ton-anywhere ton-anywhere Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For record(no need to answer)

native-tls https failing on master

tests command

cargo test -F async-https-native-tls -- --no-capture --test-threads=1

failing tests

#[tokio::test]
#[cfg(all(feature = "native-tls", not(feature = "rustls"), feature = "tokio-native-tls"))]
async fn test_https() {
    assert_eq!(get_status_code(bitreq::get("https://example.com")).await, 200);
    assert_eq!(get_status_code(bitreq::get("https://example.com")).await, 200);
}

#[tokio::test]
#[cfg(all(feature = "native-tls", not(feature = "rustls"), feature = "tokio-native-tls"))]
async fn test_https_with_client() {
    setup();
    let client = bitreq::Client::new(1);
    let response = client.send_async(bitreq::get("https://example.com")).await.unwrap();
    assert_eq!(response.status_code, 200);
}

Error

Error::HttpsFeatureNotEnabled

Evidence

native-tls-test-fail rustls-test-success

Why

As i've mentioned, https for native-tls is currently not working, it's feature gated for tokio-rustls ONLY. This error is triggered on the AsyncConnection::new connection.rs#L275

image

Fix

This is corrected it the draft PR I've mentioned above, together with the custom root certificate support. Will merge on this PR or open a new one depending on the answer above

pub(crate) fn append_certificate(mut self, certificate: &[u8]) -> Result<Self, Error> {
let mut certificates = self.inner;
certificates
.add(&rustls::Certificate(certificate.to_owned()))
.map_err(Error::RustlsAppendCert)?;
self.inner = certificates;
Ok(self)
}

#[cfg(feature = "rustls")]
pub(crate) fn with_root_certificates(mut self) -> Self {
let mut root_certificates = self.inner;

// Try to load native certs
#[cfg(feature = "https-rustls-probe")]
if let Ok(os_roots) = rustls_native_certs::load_native_certs() {
for root_cert in os_roots {
// Ignore erroneous OS certificates, there's nothing
// to do differently in that situation anyways.
let _ = root_certificates.add(&rustls::Certificate(root_cert.0));
}
}

#[cfg(feature = "rustls-webpki")]
{
#[allow(deprecated)]
// Need to use add_server_trust_anchors to compile with rustls 0.21.1
root_certificates.add_server_trust_anchors(TLS_SERVER_ROOTS.iter().map(|ta| {
rustls::OwnedTrustAnchor::from_subject_spki_name_constraints(
ta.subject,
ta.spki,
ta.name_constraints,
)
}));
}
self.inner = root_certificates;
self
}
}
Loading