fix: pin rustls to a single crypto provider preventing panics - #543
Closed
surpher wants to merge 1 commit into
Closed
fix: pin rustls to a single crypto provider preventing panics#543surpher wants to merge 1 commit into
surpher wants to merge 1 commit into
Conversation
Contributor
|
Leave this one with me, it looks like it breaks things. I've had lots of issues with |
Contributor
|
I have released FFI 0.5.6 which hopefully fixes this issue. I'll close this PR. |
Member
|
Awesome, thanks Ron |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
disclaimer: The following PR description was prepared with help of Claude AI.
Before accepting these suggested changes, verify it doesn't break other
libpact_ffiusers.Problem
Starting a TLS mock server panics and then aborts the host process.
rustlscannot pick a crypto provider because both of its backend features are enabled:The panic then poisons a lock inside
pact_ffi, and the nextpactffi_free_pact_handleunwraps thePoisonErrorfrom a context that cannot unwind, which turns a recoverable error into a hard abort:For an FFI consumer this is fatal rather than merely a failed test: the whole test host dies, taking unrelated tests with it. It reproduces through the FFI on
libpact_ffi-v0.5.5(andv0.5.4) onaarch64-apple-darwinviapactffi_create_mock_server_for_transportwith an HTTPS transport, and equally viapactffi_get_tls_ca_certificate.Root cause
Both providers get enabled by feature unification, because the workspace straddles two
reqwestmajors that default to different backends:pact_matching,pact_modelsrustls-tls-native-roots__rustls-ring→ ringpact_ffi,pact_verifier,pact_consumerrustls__rustls-aws-lc-rs→ aws-lc-rspact_mock_server2.2.4 — which implements the TLS mock server — also depends onrustls,tokio-rustlsandhyper-rustlswithringdirectly. Cargo unions all of it, sorustlsends up with both:In reqwest 0.13, the
rustlsfeature hard-wires the backend, whereasrustls-no-provideris the same feature set minus the forced backend:Change
Switch the three reqwest 0.13 dependencies from
rustlstorustls-no-provider, leavingring— already required bypact_mock_server— as the single provider:pact_ffi/Cargo.tomlpact_verifier/Cargo.tomlpact_consumer/Cargo.tomlBecause the two features differ only in the forced backend,
rustls-platform-verifierstays enabled and certificate verification behaviour is unchanged.Result:
aws-lcdisappears from thepact_ffidependency graph entirely, which also drops theaws-lc-sysC build.Cargo.lockloses exactly two dependency edges (aws-lc-rsfromquinn,quinnfromreqwest); no packages are added or removed.Verification
Built
pact_ffiforaarch64-apple-darwin,aarch64-apple-iosandaarch64-apple-ios-sim, then ran the full test suite of a Swift FFI consumer (PactSwiftMockServer) against the resulting static libraries:CryptoProviderpanics and 2 aborts per run.pactffi_get_tls_ca_certificateand HTTPS-transport mock server cases now pass.nmreports noaws_lcsymbols andring_coresymbols present in all three archives.I have not run the pact-reference Rust test suite itself — worth a CI run, particularly anything covering
pact_verifier's HTTPS client, since that is the code path whose provider selection now comes from the process-level default rather than from reqwest's feature.Test logs after applying changes and re-building
libpact_ffibinaries to use withPactSwiftMockServer:pact-swift-mock-server_tests.log