feat(websocket): share port with HTTP handler#3509
Conversation
Add WithFallbackHTTPHandler so /ws and /tls/ws listeners can serve a caller-supplied http.Handler for every request that is not a WebSocket upgrade. Lets a libp2p node share its TCP port with a regular HTTP/HTTPS site (censorship resistance, single-port deployments, HTTP-native retrieval endpoints next to libp2p WebSockets). - option + listener wiring with handshake-timer disarm so HTTP/2 streams and HTTP/1.1 keep-alive survive past the 15s default - HTTP/2 over TLS via http.Server.ServeTLS ALPN; HTTP/2 cleartext via h2c.NewHandler wrap when the listener is plaintext - tcpreuse IsHTTP gains PRI so the h2c connection preface routes to the HTTP demux branch - transport stays interop-maximal: any HTTP version on any listener; application-level version policy is the wrapped handler's call - 9 fallback handler tests, modern-browser wss:// flow with raw h2 framer SETTINGS check, h2 ext-CONNECT regression, h2 keep-alive disarm
Two test handlers intentionally ignore one of (w, r); revive flagged them in PR #3509's go-check job.
negotiatingConn.Unwrap read and wrote stopClose without synchronization and treated context.AfterFunc's stop returning false as a timeout. Under HTTP/2 multiplexing, several streams on one TCP connection share a single *negotiatingConn, so concurrent Unwrap calls raced on stopClose and the losers silently dropped their requests. Wrap the disarm in sync.Once and record the AfterFunc outcome so all callers observe the same result. Add a unit test on the disarm contract and an integration test that drives concurrent h2 streams through the fallback handler, both run under -race.
|
@MarcoPolo @sukunrt kind ping to put this on the top of review queue. The foundations of It also adds opt-in |
Pulls in Boxo v0.40.0, go-libp2p-kad-dht v0.40.0, cheggaaa/pb v3, and the dag --local-only / migration fetcher work from master. Dep conflicts resolved by keeping our HTTPProvider-required pins on top of master's baseline: - certmagic v0.25.3 (pulled in by p2p-forge v0.9.0) - p2p-forge v0.9.0 - go-libp2p v0.48.1-0.20260515215300-a72c0588b088 (pin for libp2p/go-libp2p#3509, still open; needed for websocket.WithFallbackHTTPHandler) Other transitive bumps follow from `make mod_tidy`. AutoTLS canary + HTTPProvider CLI tests pass on the merged tree.
| hostPort := startWSSListener(t, handler) | ||
|
|
||
| // Force ALPN to h2 only, so the connection MUST be HTTP/2 or fail. | ||
| resp, err := httpsClient([]string{"h2"}).Get("https://" + hostPort + "/") |
There was a problem hiding this comment.
I think you need an extended connect request here to make it fail if gorilla/ws implements h2 websocket support.
There was a problem hiding this comment.
Good catch @sukunrt, you're right. A plain GET never trips IsWebSocketUpgrade. Improved and it now sends a real extended CONNECT.
One wrinkle worth noting: the bundled h2 server resets the stream before the handler ever sees an extended CONNECT unless it advertises ENABLE_CONNECT_PROTOCOL (golang/go#71128), which is gated by GODEBUG=http2xconnect=1 and read once at startup. So the test re-execs itself in a child with that set, sends the extended CONNECT over raw frames, and asserts it reaches the fallback handler. Confirmed it goes red if dispatch ever routes extended CONNECT to the WS upgrade path.
See 9dce1ca. Not a fan, but unsure if this could be done in a better way, given golang limitations.
Strengthen the HTTP/2-never-reaches-WS test so it actually proves the invariant it documents. A plain h2 GET can never be a WebSocket upgrade, so the old test would not have caught a gorilla/websocket bump that teaches IsWebSocketUpgrade about RFC 8441 extended CONNECT. - send a real extended CONNECT (:method=CONNECT, :protocol=websocket) over raw h2 frames and assert it reaches the fallback handler - re-exec the test in a child with GODEBUG=http2xconnect=1, since the bundled h2 server otherwise rejects extended CONNECT before any handler runs and the setting is read once at startup - bound the child with a context timeout, merge into any inherited GODEBUG, and require the child to report a pass so a zero-match -test.run cannot pass silently
| // On a plaintext /ws listener with a fallback handler, wrap the handler | ||
| // so the same listener also accepts HTTP/2 cleartext (h2c). Reverse | ||
| // proxies that talk h2c to backends (Caddy, Traefik, nginx with the | ||
| // right config) get connection multiplexing for free; reverse proxies | ||
| // that only speak h1 to backends keep working through the same handler. | ||
| // On a /tls/ws listener we don't need this: h2 is negotiated via ALPN | ||
| // inside http.Server.ServeTLS. | ||
| if !parsed.isWSS && fallbackHTTPHandler != nil { | ||
| ln.server.Handler = h2c.NewHandler(ln, &http2.Server{}) | ||
| } | ||
| // Note on RFC 8441 (WebSocket-over-HTTP/2): | ||
| // | ||
| // We do NOT explicitly enable the HTTP/2 SETTINGS_ENABLE_CONNECT_PROTOCOL | ||
| // flag here, because Go's bundled http2 stack and golang.org/x/net/http2 | ||
| // do not yet expose a per-server API for it (golang/go#71128). The flag | ||
| // is gated by a process-global GODEBUG=http2xconnect=1, which a library | ||
| // must not set on its own. | ||
| // | ||
| // The dispatch in ServeHTTP is, however, written to upgrade | ||
| // transparently: it defers the "is this a WebSocket upgrade?" decision | ||
| // to ws.IsWebSocketUpgrade. When upstream Go exposes the setting and | ||
| // gorilla/websocket adds server-side ext-CONNECT support (most likely | ||
| // by extending IsWebSocketUpgrade and Upgrader.Upgrade to recognise | ||
| // `:method=CONNECT, :protocol=websocket`), this listener will accept | ||
| // WS-over-HTTP/2 with no code change — only a gorilla bump and a | ||
| // future Go upgrade. |
There was a problem hiding this comment.
You can structure this code to be less go defaults dependent by explicitly configuring the http2 server using http2.ConfigureServer
And then we can just opt in to Extended Connect always, and don't have to depend on the godebug for tests.
There was a problem hiding this comment.
Thanks, I split this into the two things you raised:
On making it less go-defaults dependent: done in 0bf17d2. It configures the server explicitly, dropping the x/net/http2/h2c wrapper for net/http Protocols so one server handles HTTP/1.1, HTTP/2, and h2c, following the http2.ConfigureServer pattern. It also adds WithHTTPServerConfig(func(*http.Server)) so callers set timeouts and HTTP/2 options on a transport-owned server.
On opting into Extended Connect always: there is no per-server switch, and we should not advertise it anyway. In golang.org/x/net/http2 (v0.50.0 and the latest v0.55.0) a single package-global decides whether the server sends SETTINGS_ENABLE_CONNECT_PROTOCOL, flipped only by GODEBUG=http2xconnect=1 at startup; http2.ConfigureServer, http2.Server, and net/http.HTTP2Config expose no field for it (golang/go#71128 turned it off by default). Even with a switch, advertising it would break browser wss://: gorilla has no server-side RFC 8441, so dispatch (via ws.IsWebSocketUpgrade, which reads only HTTP/1.1 Upgrade headers) would route a browser's :method=CONNECT, :protocol=websocket to the fallback handler instead of upgrading. TestModernBrowserWSSFlow pins the absence of the flag; the GODEBUG re-exec in the ext-CONNECT test only simulates that future to prove dispatch stays correct.
Rationale now recorded in bb289fc.
|
@lidel We can merge it after fixing the verbose comments. But I think this design would be better: #3509 (comment) |
Rename the internal fallbackHTTPHandler field to httpHandler to match the option.
Let callers tune the http.Server that serves the fallback handler (timeouts, HTTP/2 settings), following the http2.ConfigureServer pattern: the function configures a transport-owned server, and the transport sets Handler, ConnContext, and TLSConfig. Apply safe defaults (ReadHeaderTimeout, IdleTimeout) when only WithHTTPHandler is set. Serve HTTP/2 cleartext through net/http Protocols, so one server handles HTTP/1.1, HTTP/2, and h2c uniformly, and the per-connection context reaches h2c streams.
The design rationale for sharing the WebSocket port: A /tls/ws listener looks like a normal HTTPS endpoint that a single probe cannot fingerprint as libp2p, so it is harder to censor. The server intentionally does not advertise WebSocket-over-HTTP/2 (RFC 8441): gorilla/websocket has no server-side support, so it omits SETTINGS_ENABLE_CONNECT_PROTOCOL and browsers fall back to HTTP/1.1, where the upgrade works.
8a828bd to
bb289fc
Compare
|
Thanks @sukunrt, I went through feedback and did my best:
One thing I could not do the way you suggested: turning Extended Connect on by default. There is no per-server switch for it today, and turning it on would break browser Happy to change anything if you see a better way. Thanks again for the careful look. |
Thanks @lidel . I just assumed there would be such an option. |
Track the released go-libp2p instead of the pre-merge PR commit now that libp2p/go-libp2p#3509 has landed on master. The HTTPProvider fallback handler comment also drops the old WithFallbackHTTPHandler name in favor of the merged WithHTTPHandler API.
2 participants
Adds
websocket.WithHTTPHandler(http.Handler)so a/wsor/tls/wslistener answers non-WebSocket requests with the supplied handler instead of 404, on the same TCP port, pluswebsocket.WithHTTPServerConfig(func(*http.Server))to configure that server's timeouts and HTTP/2 settings.👉 Opt-in, backward-compatible, default behaviour is unchanged, but go-libp2p gains important regression tests.
Two uses
/tls/wsport. HTTP retrieval clients (boxo/bitswap/network/httpnet) fetch blocks directly over HTTPS using the AutoTLS cert. Improved interop of IPFS stack.Routing
WebSocket upgrades go to libp2p as before; everything else goes to the handler. The handler runs over HTTP/1.1 and HTTP/2 on TLS listeners (ALPN inside
http.Server.ServeTLS) and over HTTP/1.1 and HTTP/2 cleartext on plaintext listeners (net/httpProtocolswithSetUnencryptedHTTP2). Also intcpreuse:IsHTTPmatches thePRIh2c preface so it routes to the HTTP demux branch.Important
Real-world example in ipfs/kubo#11333, where kubo uses both opt-in funcs to expose its trustless gateway on the AutoTLS-secured
/wsand/tls/wsTCP ports, on one Let's Encrypt cert:WithHTTPHandlerinstalls the trustless gateway handler (h2 required over TLS, h1+h2c allowed cleartext).WithHTTPServerConfigsets streaming-safe timeouts and HTTP/2 limits, derivingWriteByteTimeout/SendPingTimeoutfromGateway.RetrievalTimeoutso the gateway's own 504 fires before the connection is dropped.An in-process AutoTLS canary (Pebble + p2p-forge) exercises the full path end to end.
Regression tests
👉 tests added here protect ecosystem from silently breaking
/ws.gorilla/websocket has no HTTP/2 / RFC 8441 server support today. WebSocket-over-h2 needs extended CONNECT (
:method=CONNECT, :protocol=websocket); gorilla'sUpgrader.Upgradestill reads HTTP/1.1 hijack-based headers, and Go's h2 stack does not advertiseSETTINGS_ENABLE_CONNECT_PROTOCOL(gated on process-globalGODEBUG=http2xconnect=1; per-server API tracked ingolang/go#71128). Two tripwires added by this PR catch any future upstream shift so go-libp2p maintainers can deliberately embrace WS-over-h2 or pin the dispatch:TestFallbackHTTPHandler_HTTP2NeverReachesWSPath: every HTTP/2 request reaches the fallback handler, never the WebSocket hijack path. Fails if a future gorilla release extendsIsWebSocketUpgradeto recognise ext-CONNECT.TestModernBrowserWSSFlow: the server's initial h2 SETTINGS frame omitsSETTINGS_ENABLE_CONNECT_PROTOCOL. Browsers rely on its absence to fall back to HTTP/1.1 forwss://; the test fails if Go ever flips this flag on by default.Plus a concurrency pin for the new fallback path:
TestFallbackHTTPHandler_KeepAlive+TestNegotiatingConnUnwrapConcurrent: the handshake-timer is disarmed once a fallback request arrives, safely under concurrent HTTP/2 streams sharing one*negotiatingConn.