Skip to content

fix(node:https,http2): accept Buffer-typed key/cert PEMs (#2132) - #2184

Merged
proggeramlug merged 1 commit into
mainfrom
worktree-fix-2132-http-byte-diffs
May 28, 2026
Merged

fix(node:https,http2): accept Buffer-typed key/cert PEMs (#2132)#2184
proggeramlug merged 1 commit into
mainfrom
worktree-fix-2132-http-byte-diffs

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Summary

Towards #2132. https.createServer({ key, cert }) and http2.createSecureServer rejected the Node-idiomatic Buffer form of key/cert (the shape fs.readFileSync('key.pem') returns when no encoding is supplied), printing "[node:https] no recognized PEM private key" and refusing to listen.

Why

The two helpers round-trip their options object through JSON.stringifyserde_json.as_str(). A Buffer JSON-serializes as {"type":"Buffer","data":[...]}, so .as_str() returns None, the PEM buffer becomes empty, and the request bottoms out in the "no recognized PEM private key" branch. The node-core radar (#800) surfaced this as the diff-bucket cause for the three https tests cited in #2132:

  • test-https-agent-abort-controller.js
  • test-https-agent-keylog.js
  • test-https-byteswritten.js

Fix

Add a shared json_value_to_pem_bytes helper in tls.rs that recognizes both the Node Buffer shape and a bare numeric array, and decodes them to bytes. Both option-parsers now thread byte buffers (not String) end-to-end — PEM is opaque to the TLS parsers, so there's no reason to require valid UTF-8.

Verified

  • cargo test --release -p perry-ext-http-server --lib — 12 passed (4 new tests cover string PEM, Buffer-shape JSON, bare numeric array, empty/unknown fallbacks).
  • cargo fmt --all -- --check clean.
  • Direct re-run of the three diff tests against the local radar staging:
    • test-https-agent-abort-controller.js: was diff, now pass (both runtimes silent, exit 0).
    • test-https-agent-keylog.js: PEM step now succeeds; next blocker is rustls UnsupportedCertVersion on the Node test fixture (unrelated, distinct follow-up).
    • test-https-byteswritten.js: PEM step now succeeds; remaining diff is a missing console.log('ok') reachable code path (unrelated, distinct follow-up).

Scope

This is the first cluster from #2132 — it closes the PEM-Buffer source of diffs. Remaining buckets surfaced by the radar (server method dispatch, agent getName / addRequest, the UnsupportedCertVersion cluster) will be follow-up PRs.

Test plan

  • cargo test --release -p perry-ext-http-server --lib
  • cargo fmt --all -- --check
  • Local node-core radar comparison on the three cited diff tests
  • CI green

`https.createServer({ key, cert })` and `http2.createSecureServer`
parsed their options object via `JSON.stringify` → `serde_json` →
`.as_str()`. Node-idiomatic callers pass PEMs as Buffers
(`fs.readFileSync('key.pem')` with no encoding), which round-trip
through that path as `{"type":"Buffer","data":[...]}` — losing the
`.as_str()` extraction and falling through to the
`"no recognized PEM private key"` branch with an empty PEM buffer.

Add a shared `json_value_to_pem_bytes` helper that recognizes the
Node Buffer shape and a bare numeric array, so both helpers accept
either string- or Buffer-typed PEMs. Threads byte buffers (not
`String`) end-to-end since PEM is opaque to the TLS parsers.

This was the diff-bucket cluster surfaced by the node-core radar
for the three https tests cited in #2132 (agent-abort-controller,
agent-keylog, byteswritten); after the fix, agent-abort-controller
flips to pass, the other two move past the PEM error to surface
unrelated downstream gaps (cert version, console output).

Includes unit tests covering string PEM, Buffer-shape JSON, bare
numeric array, and the empty/unknown fallbacks.
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.

1 participant