Skip to content

RDBC-1092 Support PKCS#12 client certificates on Bun - #583

Merged
M4xymm merged 3 commits into
ravendb:v7.2from
M4xymm:RDBC-1092
Sep 15, 2026
Merged

M4xymm merged 3 commits into
ravendb:v7.2from
M4xymm:RDBC-1092

Conversation

@M4xymm

@M4xymm M4xymm commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

Issue link

https://issues.hibernatingrhinos.com/issue/RDBC-1092

Description

Bun's fetch silently ignores the tls.pfx option (oven-sh/bun#41958, oven-sh/bun#17543), so a PKCS#12 certificate configured through authOptions was never presented and the request went out uncertified. Its node:https implementation does honour pfx (oven-sh/bun#14417), so those requests are routed there instead.

src/Utility/BunHttpUtil.ts installs a fetch-compatible transport backed by node:https as the request fetcher (the same seam conventions.customFetch uses) when a PKCS#12 archive is configured on Bun. PEM material keeps using Bun's tls fetch option, which works. The transport is built lazily, once per executor, closed in dispose(), and not installed when conventions.customFetch owns the transport, the same rules the Deno Deno.HttpClient path follows. Whether the certificate needs it is decided once, in the executor constructor, not per request.

Before Bun 1.4.0 neither transport presents a client certificate (node:https ignored the certificate and the ca as well; verified on 1.3.6 vs 1.4.0), so DocumentStore.initialize() now throws an actionable error there instead of failing per request. An unparseable process.versions.bun is not treated as an old runtime; the request path decides.

RavenCommand.send passes a node:stream Readable body through when the fetcher declares acceptsNodeStreamBody, so streamed attachment uploads keep working on that path. The global fetch and a user-supplied customFetch do not declare it, so their behaviour is unchanged.

Also drops the console.warn from PfxCertificate.toBunTlsOptions (the method itself stays: it is public API, and the executor no longer calls it for a PKCS#12 archive on Bun) and documents the Bun runtime in README.md, including the pre-existing Changes API gap (Bun's built-in WebSocket ignores the ws package's TLS options — upstream oven-sh/bun#31396, and for the PKCS#12 half oven-sh/bun#42322).

Transport design notes

  • PKCS#12 detection reads certificate.toSocketOptions().pfx rather than instanceof PfxCertificate: the package ships CommonJS and ESM builds with separate class identities, and a certificate created from the other build would silently fail the instanceof check and go out uncertified. The node:stream Readable check on request bodies is duck-typed for the same reason.
  • Request bodies: strings and binary go straight through; FormData, Blob, URLSearchParams and web streams are encoded by the platform Response, which also yields the content-type (including the multipart boundary the batch command relies on fetch to generate). A node:stream Readable is piped, so a large attachment does not have to fit in memory first.
  • content-length: node:http would otherwise fall back to chunked transfer encoding. The transport mirrors fetch and always sends a length, including an explicit content-length: 0 for body-less non-GET/HEAD requests. A streamed body has no length and stays chunked, exactly as undici sends it on Node.
  • accept-encoding is built from what the runtime's zlib can decode: gzip, deflate, br, plus zstd where createZstdDecompress exists (Node >= 22.15, Bun >= 1.2; an older runtime gets the raw bytes). A decoded response drops content-encoding and content-length, as fetch does. Decompression runs through stream.pipeline, so a source error tears the decompressor down instead of leaving the body hanging.
  • Body-less responses (HEAD, 204, 205, 304) are decided before any decompressor exists: the Response constructor rejects a body for those statuses, and a gunzip on empty input is an unhandled Z_BUF_ERROR.
  • Abort: the abort listener stays registered until the request closes, not just until the headers land, so an abort during a streamed response body tears the connection down too. The body then fails with the signal's AbortError (RequestExecutor branches on error.name === "AbortError" for its timeout path) rather than an ECONNRESET.
  • Redirects are not followed: the RavenDB API does not use them, and silently re-issuing a request elsewhere is worse than surfacing the 3xx.
  • BunTlsOptions.pfx stays in the type for shape compatibility with Node's TLS options; its doc now only warns that Bun's fetch ignores it.

Verification run locally: tsc --noEmit clean; test/Utility/BunHttpUtilTest.ts + test/Executor/RequestExecutorBunTests.ts 38 passing across 5 consecutive runs (the transport tests no longer use timers); eslint clean on every file this PR touches; npm run check-exports passes. npm run lint over the whole repo reports 4 errors, all in the generated, gitignored test/cloudflare-nitro/.nitro/types/ output — unrelated to this change. There is no npm run build script (prepare runs tshy); npm run check-imports was not run because it rewrites src/index.ts. End-to-end verified on Bun 1.4.3 against RavenDB 7.2.6 over a PFX certificate (HttpsTest, and a DocumentStore with useHttpDecompression = true receiving content-encoding: zstd).

Type of change

  • Bug fix
  • Regression bug fix
  • Optimization
  • New feature
  • Sync with the C# client (version x.y.z -> x.y.z)
  • Dependency / tooling / CI update

Target branch and backports

  • This PR targets the correct release branch (e.g. v7.2, v7.1, v7.0, v6.0)
  • The change needs to be ported to other release branches. Please list them.
  • No other release branch is affected

How risky is the change?

  • Low
  • Moderate
  • High
  • Not relevant

Backward compatibility

  • Non breaking change
  • Ensured. Please explain how has it been implemented?
  • Breaking change (public API, exported types, default behavior). Please describe the migration path.
  • Not relevant

Server compatibility

  • Works with all RavenDB server versions covered by CI
  • Requires a minimum server version. Please specify which one and make sure the tests are gated accordingly.
  • Not relevant

Affected runtimes

  • Node.js
  • Bun
  • Deno
  • Cloudflare Workers
  • Not runtime specific

Public API

  • New or changed public API. New types are exported from src/index.ts (npm run check-exports passes).
  • Version bump: package.json and CLIENT_VERSION in src/Http/RequestExecutor.ts (sync / release PRs only)
  • No public API changes

Documentation update

  • README.md has been updated
  • No documentation update is needed

Testing by Contributor

  • Tests have been added that prove the fix is effective or that the feature works
  • Existing tests verify the correct behavior
  • It has been verified by manual testing
  • npm run lint, npm run build, npm run check-exports and npm run check-imports pass locally
  • Tests have been run locally against a RavenDB server (RAVENDB_TEST_SERVER_PATH / RAVENDB_SERVER_VERSION)
  • Runtime-specific changes have been verified on the affected runtime (Bun / Deno / Cloudflare Workers)

Dependencies

  • New or updated runtime dependency. Please explain why it is needed and confirm it works on all affected runtimes.
  • Dev dependency only
  • No dependency changes

Is there any existing behavior change of other features due to this change?

  • Yes. Please list the affected features/subsystems and provide appropriate explanation
  • No

Two, both on Bun only:

  1. PKCS#12 on Bun now goes through node:https rather than fetch. Previously the request went out with no client certificate and the server rejected it; it now authenticates. PEM on Bun is unchanged.
  2. PKCS#12 on Bun older than 1.4.0 now throws at DocumentStore.initialize(). Previously every request silently went out uncertified. The error names the openssl pkcs12 -in cert.pfx -out cert.pem -nodes conversion and type: "pem" as the way to stay on an older Bun.

Bun's fetch silently ignores the `tls.pfx` option (oven-sh/bun#41958,
oven-sh/bun#17543), so a PFX certificate configured through authOptions
was never presented and the request went out uncertified. Its node:https
implementation does honour `pfx` (oven-sh/bun#14417), so route those
requests there instead.

BunHttpUtil installs a fetch-compatible transport backed by node:https as
the request `fetcher` - the same seam conventions.customFetch uses - when
a PKCS#12 archive is configured on Bun. PEM material keeps using Bun's
`tls` fetch option, which works. The transport covers what the client asks
of fetch: methods, headers, string/binary/FormData/stream request bodies,
streamed response bodies, AbortSignal, and content-encoding decompression.
It is built lazily, once per executor, and closed in dispose().

Before Bun 1.4.0 neither transport presents a client certificate (node:https
ignored the certificate and the ca as well), so DocumentStore.initialize()
now throws an actionable error there instead of failing per request.

RavenCommand.send passes a node:stream Readable body through when the
fetcher declares it accepts one, so streamed attachment uploads keep
working on that path.

Drops the now-obsolete console.warn from PfxCertificate.toBunTlsOptions
and documents the Bun runtime, including the pre-existing Changes API gap
(Bun's built-in WebSocket ignores the ws package's TLS options).
@M4xymm M4xymm self-assigned this Sep 11, 2026
@M4xymm
M4xymm requested a review from kalczur September 11, 2026 12:53
Route the response through stream.pipeline instead of res.pipe(decompressor):
pipe() forwards data only, so when the connection dropped or the request was
aborted mid-way through a compressed body the gunzip stream never ended or
errored and the body never settled. Reproduced on both Node and Bun.

Send an accept-encoding header built from what zlib can decode (gzip, deflate,
br, and zstd where available) unless the caller set one. node:http sends none
by default, unlike fetch, so the server never compressed and the decoding path
was dead code; the executor still opts out with `identity` when
conventions.useHttpDecompression is off.

Decide the body-less HEAD/204/205/304 case before building a decompressor. A
gunzip fed the empty input of a HEAD reply that carries content-encoding failed
with an unhandled Z_BUF_ERROR.

Surface the signal's AbortError on the body when an abort lands mid-way, as
fetch does, instead of the bare ECONNRESET the socket teardown produces, so the
executor's timeout path still recognises it.

Decide requiresNodeHttpsTransport once in the RequestExecutor constructor
rather than on every request.

Add a PKCS#12 client-certificate leg to the Bun workflow (server 7.2): with
RAVENDB_TEST_CLIENT_CERT_PATH empty, TestUtil falls back to the server's PFX as
the client certificate, so the node:https transport is exercised against a real
server in CI.
Comment thread src/Auth/Certificate.ts Outdated
Comment thread test/Utility/BunHttpUtilTest.ts Outdated
…terministic

Comments describe current behaviour only; the rationale moved to the PR description. The connection-drop and abort tests sequence the fault after fetch() resolves instead of on a timer, and the hang guard and settle wait are gone.
@M4xymm
M4xymm requested a review from kalczur September 15, 2026 12:08
@M4xymm
M4xymm merged commit 7de8c57 into ravendb:v7.2 Sep 15, 2026
24 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.

2 participants