Skip to content

fix(pact-ffi): pactffi_create_mock_server_for_transport is unable to start a TLS mock server#530

Open
rholshausen wants to merge 1 commit intomasterfrom
fix-ffi-tls-mockserver
Open

fix(pact-ffi): pactffi_create_mock_server_for_transport is unable to start a TLS mock server#530
rholshausen wants to merge 1 commit intomasterfrom
fix-ffi-tls-mockserver

Conversation

@rholshausen
Copy link
Copy Markdown
Contributor

fix(pact_ffi): pactffi_create_mock_server_for_transport now starts a TLS server when transport is "https"

Problem

Callers passing "https" to pactffi_create_mock_server_for_transport received a plain HTTP server, not an HTTPS one. The function's doc comment explicitly lists https as a supported transport value, so this was a silent regression against the documented contract.

Root cause

The function forwarded every transport string — including "http" and "https" — directly to MockServerBuilder::with_transport(...). That method looks up the transport in the plugin catalogue and stores a CatalogueEntry on the builder config. It has no effect on whether the underlying server uses TLS.

Whether TLS is used is decided later, in ServerManager::spawn_http_mock_server, which checks builder.tls_configured(). That flag is only set to true when one of the explicit TLS-configuration methods (with_tls_config, with_tls_certs, or with_self_signed_tls) has been called on the builder. Because the FFI path called none of them, "https" was silently treated as plain HTTP.

The older, now-deprecated FFI functions (create_tls_mock_server*, start_tls_mock_server*) worked correctly because they took an explicit ServerConfig and passed it straight through to the mock server. That explicit TLS branch was dropped when the replacement pactffi_create_mock_server_for_transport was introduced.

Fix

Special-case the transport string in pactffi_create_mock_server_for_transport before dispatching to the builder:

Transport Behaviour
"https" Calls builder.with_self_signed_tls() — sets tls_config on the builder, causing spawn_http_mock_server to call start_https()
"http" Passes the builder through unchanged (HTTP is the default path)
anything else Calls builder.with_transport(...) for plugin-provided transports (unchanged)

The doc comment on the function has been updated to state that "https" starts a TLS server using a self-signed certificate, and points callers to pactffi_get_tls_ca_certificate for configuring their HTTP clients.

Test

A new integration test https_mock_server_starts_and_responds_over_tls in pact_ffi/tests/tests.rs:

  1. Builds a pact with a single GET / → 200 interaction via the handle API.
  2. Starts the mock server with transport = "https" and port 0 (OS-assigned).
  3. Sends an HTTPS GET request using a reqwest client configured with danger_accept_invalid_certs(true) (the self-signed cert is not trusted by the system CA store).
  4. Asserts the response status is 200, pactffi_mock_server_matched returns true, and the mismatch list is empty.

Note on crypto provider initialisation

The test binary links both the ring backend (via pact_mock_server) and the aws-lc-rs backend (via reqwest 0.13). When both are present, rustls panics at ServerConfig::builder() unless a process-level CryptoProvider has been explicitly installed. MockServerBuilder::with_self_signed_tls is missing the provider-installation guard that with_tls_certs already has, so the test installs the ring provider explicitly at startup:

rustls::crypto::ring::default_provider().install_default().ok();

rustls has been added to pact_ffi's [dev-dependencies] with the ring feature for this purpose. This is a test-only dependency; it does not affect the shipped library. The missing guard in with_self_signed_tls should be reported as a bug upstream in pact-core-mock-server.

Files changed

  • rust/pact_ffi/src/mock_server/mod.rs — transport dispatch logic and doc update
  • rust/pact_ffi/tests/tests.rs — new integration test
  • rust/pact_ffi/Cargo.tomlrustls added to [dev-dependencies]

Copy link
Copy Markdown
Member

@mefellows mefellows left a comment

Choose a reason for hiding this comment

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

Nice, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants