Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ jobs:
toolchain: stable
cache-key: capture-v1
command: cargo build --verbose --features capture-v1
- name: tls-no-provider
toolchain: stable
cache-key: tls-no-provider
command: >-
cargo build --verbose --no-default-features --features async-client,error-tracking,capture-v1,tls-no-provider
&& ! cargo tree --package posthog-rs --edges normal --invert aws-lc-sys --no-default-features --features async-client,error-tracking,capture-v1,tls-no-provider

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.

blocking: Make the dependency-tree assertion fail closed\n\nNegating cargo tree makes any failure—including registry, dependency-resolution, or argument errors—look like proof that aws-lc-sys is absent, so this regression guard can pass silently. Please run cargo tree as a separately checked command, then inspect its successful output for aws-lc-sys.

steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
Expand Down Expand Up @@ -165,9 +171,15 @@ jobs:
- name: Unit test (error-tracking + capture-v1, blocking client)
cache-key: error-tracking-capture-v1-blocking-client
command: cargo test --verbose --no-default-features --features error-tracking,capture-v1
# Targeted at the tls-no-provider test binary only: without a
# built-in provider, constructing a client panics unless the test
# installs one first, so the rest of the suite cannot run here.
- name: Unit test (tls-no-provider, blocking client)
cache-key: tls-no-provider-blocking-client
command: cargo test --verbose --no-default-features --features tls-no-provider --test test_tls_no_provider
- name: E2E test
cache-key: e2e
command: cargo test --verbose --features e2e-test --no-default-features
command: cargo test --verbose --features e2e-test,tls --no-default-features

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.

suggestion: Update the documented E2E command\n\nCI now enables tls for E2E tests, but CONTRIBUTING.md still documents --features e2e-test --no-default-features. That leaves manual E2E runs without an HTTPS backend, so captures can fail while the test appears successful. Please update the documented command to include tls.

steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
Expand Down
11 changes: 11 additions & 0 deletions .sampo/changesets/virtuous-guardian-sampsa.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
cargo/posthog-rs: minor
---

TLS backend selection is now a feature choice. `reqwest/rustls` was previously listed unconditionally on the dependency line, so every consumer compiled and linked `aws-lc-rs` -> `aws-lc-sys` with no way to opt out. Two features now control this: `tls` (enabled by default) keeps rustls with reqwest's built-in aws-lc-rs provider, and `tls-no-provider` links rustls with no crypto provider so an application can supply its own.

Applications that already install a rustls `CryptoProvider` gained nothing from the linked one — rustls resolves the process-level provider before consulting crate features, so an app calling `ring::default_provider().install_default()` already used ring at runtime while still building and linking aws-lc-rs. `tls-no-provider` drops `aws-lc-sys` from the tree entirely, removing the C/assembly build dependency (C toolchain, cmake, nasm on some targets) that complicates cross-compilation, musl targets, and minimal build containers.

With `tls-no-provider` the application MUST install a process-level provider before constructing a client; reqwest builds its TLS connector at construction time and rustls panics if no provider is available. Cargo features are additive, so `tls-no-provider` only takes effect when nothing in the dependency graph enables `tls`.

Default users are unaffected — `tls` is in `default` and behavior is unchanged. **Breaking for `default-features = false` consumers:** TLS is no longer implied, so add `tls` (or `tls-no-provider`) explicitly. Without either, the build succeeds but HTTPS requests fail at runtime with `invalid URL, scheme is not http`.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ rust-version = "1.78.0"

[dependencies]
reqwest = { version = "0.13.2", default-features = false, features = [
"rustls",
"blocking",
"json",
"gzip",
Expand Down Expand Up @@ -39,6 +38,10 @@ zstd = { version = "0.13", optional = true }

[dev-dependencies]
dotenv = "0.15.0"
rustls = { version = "0.23", default-features = false, features = [
"ring",
"std",
] }
ctor = "1.0.12"
tokio = { version = "1", features = ["full"] }
httpmock = "0.7"
Expand All @@ -48,12 +51,14 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] }
eyre = "0.6.12"

[features]
default = ["async-client", "error-tracking"]
default = ["async-client", "error-tracking", "tls"]
e2e-test = []
async-client = ["tokio"]
capture-v1 = ["brotli", "zstd"]
test-harness = []
error-tracking = ["dep:backtrace", "dep:findshlibs"]
tls = ["reqwest/rustls"]
tls-no-provider = ["reqwest/rustls-no-provider"]

[workspace]
members = [".", "compliance/adapter"]
37 changes: 37 additions & 0 deletions tests/test_tls_no_provider.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#![cfg(feature = "tls-no-provider")]
//! Bring-your-own-provider TLS: with `tls-no-provider` (and without `tls`)
//! the SDK links no rustls crypto provider, so the application must install
//! a process-level provider before constructing a client. reqwest builds its
//! TLS connector at client construction time and panics inside rustls if no
//! provider is available, so successfully constructing a client is the
//! regression check here.

use posthog_rs::{ClientOptions, ClientOptionsBuilder};

fn install_ring_provider() {
// Ignore the result: `install_default` errs if a process-level provider
// is already installed (e.g. by another test in this binary).
let _ = rustls::crypto::ring::default_provider().install_default();
}

fn options() -> ClientOptions {
ClientOptionsBuilder::default()
.api_key("phc_test_token".to_string())
.host("https://eu.i.posthog.com".to_string())
.build()
.unwrap()
}

#[cfg(not(feature = "async-client"))]
#[test]
fn blocking_client_builds_with_installed_provider() {
install_ring_provider();
let _client = posthog_rs::client(options());
}

#[cfg(feature = "async-client")]
#[tokio::test]
async fn async_client_builds_with_installed_provider() {
install_ring_provider();
let _client = posthog_rs::client(options()).await;
}