Skip to content

fix: read the TLS policy per request instead of snapshotting it - #3

Merged
jrosskopf merged 1 commit into
mainfrom
fix/tls-policy-per-request
Sep 10, 2026
Merged

jrosskopf merged 1 commit into
mainfrom
fix/tls-policy-per-request

Conversation

@jrosskopf

@jrosskopf jrosskopf commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

HttpParams captured HttpTlsPolicy in its constructor, so a client built before a setting changed kept the old policy for its whole life.

That breaks exactly the ordering the consumer's docs prescribe:

ATTACH 'https://on-prem/odata/' AS s (TYPE odata);   -- catalog client built HERE
SET erpl_ca_cert_file = '/path/to/corporate-ca.pem'; -- never reaches it
SELECT * FROM s.Orders;                              -- still fails the handshake

ATTACH-then-configure is the natural order, and it silently did nothing.

Fix

The policy is read when each request's client is created. The two snapshot fields are removed rather than left in place — nothing ever set them explicitly (they were only ever seeded from the policy and read once), so leaving them would just preserve the trap.

Cost is a mutex-guarded read of two fields per request, against a TLS handshake.

Verified end to end

case result
expired / self-signed / wrong-host certificate rejected
publicly trusted certificate 200
opt-out engaged after a request already ran 200 — the change now takes effect
verification re-enabled mid-session rejected again

That last pair is the regression this fixes; the middle two guard against over-correcting.

Consumer side: DataZooDE/erpl-web#142 adds test/sql/tls_enforcement.test covering all of it, opt-in via ERPL_TEST_TLS_ENFORCEMENT so the default run does not depend on badssl.com being reachable.

Fixes DataZooDE/erpl-web#138.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

HttpParams captured HttpTlsPolicy in its constructor, so a client built
before a setting changed kept the old policy for its whole life. A
long-lived client - an ATTACHed catalog, say - therefore never saw a CA
bundle set afterwards, which is exactly the order the consumer's docs tell
users to follow.

The policy is now read when each request's client is created, and the two
snapshot fields are gone rather than left as a trap.
@jrosskopf
jrosskopf merged commit 81a4eb2 into main Sep 10, 2026
4 checks passed
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.

fix: TLS settings do not reach already-constructed HTTP clients, contradicting docs/TLS.md

1 participant