node:tls: fix getCipher().version, getEphemeralKeyInfo(), client STARTTLS, and checkServerIdentity exception handling - #32824
Open
robobun wants to merge 6 commits into
Open
node:tls: fix getCipher().version, getEphemeralKeyInfo(), client STARTTLS, and checkServerIdentity exception handling#32824robobun wants to merge 6 commits into
robobun wants to merge 6 commits into
Claude / Claude Code Review
completed
Jul 9, 2026 in 19m 55s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/runtime/socket/tls_socket_functions.rs:1039-1051 |
getEphemeralKeyInfo() reports a group on a resumed TLS 1.2 session where Node reports empty |
Annotations
Check warning on line 1051 in src/runtime/socket/tls_socket_functions.rs
claude / Claude Code Review
getEphemeralKeyInfo() reports a group on a resumed TLS 1.2 session where Node reports empty
nit: On a resumed TLS 1.2 session, `SSL_get_negotiated_group` returns the original handshake's group (BoringSSL serializes `group_id` into the session, `kGroupIDTag` in `ssl_asn1.cc`), so Bun would report `{type:'ECDH', name:'X25519', size:253}` where Node — reading `s3->peer_tmp`, which is only set when a ServerKeyExchange is processed — returns the empty three-key object. An early return on `ffi::SSL_session_reused(ssl) != 0` (already declared in this module) would match Node. Not blocking: na
Loading