Skip to content

test: handshake a wire-format ALPNProtocols string - #139

Open
SebTardif wants to merge 2 commits into
openclaw:mainfrom
SebTardif:fix/serve-string-alpn
Open

SebTardif wants to merge 2 commits into
openclaw:mainfrom
SebTardif:fix/serve-string-alpn

Conversation

@SebTardif

@SebTardif SebTardif commented Oct 7, 2026 •

Copy link
Copy Markdown

What does this PR do?

Bun.serve keeps a string ALPNProtocols as the OpenSSL wire list. A string such as "\x02h2" still negotiates h2. A plain "http/1.1" is not rewritten into a single protocol name.

Length-prefixing every string advertised a different protocol (03 02 68 32 for "\x02h2"). Checking server.port > 0 did not catch that.

How did you verify your code works?

Release build of this tree, test/js/bun/http/serve-alpn-string.test.ts.

Before restoring the wire-list path:

error: TLSV1_ALERT_NO_APPLICATION_PROTOCOL
(fail) Bun.serve forwards a wire-format ALPNProtocols string
(fail) a plain ALPNProtocols string is not a protocol name
0 pass
2 fail

After restoring it:

(pass) Bun.serve forwards a wire-format ALPNProtocols string [10.26ms]
(pass) a plain ALPNProtocols string is not a protocol name [0.20ms]
2 pass
0 fail

The command was ./build/release/bun test ./test/js/bun/http/serve-alpn-string.test.ts.

Bun.serve passed the JS string through as raw bytes. OpenSSL reads the
first byte as a length, so "http/1.1" failed with "Failed to configure
TLS ALPN protocols". A string is one protocol name. A Buffer stays the
wire format the caller already built.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

@steipete steipete left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Requesting changes: a bare string is not a single protocol name in Node's ALPN API, and this change also breaks Bun's existing wire-formatted string input.

Node v24.21.0, checked with loopback TLS handshakes in isolation:

ALPNProtocols Result
"http/1.1" or "\x08http/1.1" tls.createServer does not advertise ALPN; tls.connect throws TypeError: Must give a Buffer as first argument
["http/1.1"] Encodes as 08 68 74 74 70 2f 31 2e 31; negotiates http/1.1
Buffer.from("\x08http/1.1") or equivalent Uint8Array Preserves the encoded list; negotiates http/1.1
Buffer.from("http/1.1") Throws ERR_INVALID_ARG_VALUE for a truncated wire list

See Node's ALPN converter and socket application. HTTP/2 is not evidence for the proposed string interpretation: it replaces the list with h2, optionally adding http/1.1. Equivalent HTTP/2 checks negotiated h2 for all three supplied forms.

Separately, Bun previously forwarded a string such as "\x02h2" as the valid wire list 02 68 32. This patch emits 03 02 68 32, advertising a different protocol whose name includes the old length byte. Please preserve existing wire-string behavior; treating plain strings as names would need a separate API decision. Add a client/server handshake assertion for the negotiated protocol: server.port > 0 cannot catch this regression or detect silently omitted ALPN.

Length-prefixing every JS string turned a wire list such as "\x02h2"
into a different protocol name. Bun already accepts that string as
the OpenSSL list. A plain name is not rewritten.

The test handshakes the negotiated protocol instead of checking that
the server port is set.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@SebTardif SebTardif changed the title fix: encode a string ALPNProtocols as an OpenSSL protocol list test: handshake a wire-format ALPNProtocols string Oct 8, 2026
@SebTardif

Copy link
Copy Markdown
Author

@steipete

Please preserve existing wire-string behavior; treating plain strings as names would need a separate API decision. Add a client/server handshake assertion for the negotiated protocol.

A string ALPNProtocols is the wire list again, so "\x02h2" stays 02 68 32 and negotiates h2. The same handshake covers "\x08http/1.1". A plain "http/1.1" still fails configuration instead of being length-prefixed into a name.

This branch has not been deployed

No deployments
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