fix(ts2021): document and verify control-protocol upgrade handling#33
Conversation
Covers the /ts2021 proxy-compatibility docs, added regression test, and supporting logs. Signed-off-by: Lee Briggs <lee@leebriggs.co.uk>
There was a problem hiding this comment.
Pull request overview
This PR documents the /ts2021 control-protocol upgrade constraints (notably around non-standard upgrade semantics) and adds supporting runtime logging and a regression test to help catch proxy-compatibility issues.
Changes:
- Updated README + hosting/troubleshooting docs to clarify which frontends/proxies are compatible with
/ts2021upgrade traffic. - Added more structured logging around
/ts2021request/response headers and upstream status. - Updated the
/ts2021test to assert the proxy preserves method and upgrade-related headers when forwarding to the upstream.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a deployment note warning about /ts2021 upgrade compatibility requirements and removes the Railway deploy badge. |
| docs/troubleshooting.md | Expands troubleshooting guidance for upgrade failures and unsupported frontends. |
| docs/hosting.md | Adds a compatibility matrix and strengthens reverse-proxy guidance for preserving /ts2021 upgrade semantics. |
| cmd/serve.go | Enhances /ts2021 handler logs with method/headers and upstream response details. |
| cmd/serve_test.go | Reworks the /ts2021 test to validate POST + upgrade header preservation to the upstream. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| receivedMethod = r.Method | ||
| receivedConnection = r.Header.Get("Connection") | ||
| receivedUpgrade = r.Header.Get("Upgrade") |
There was a problem hiding this comment.
These shared variables are written from the backend httptest handler goroutine and later read from the test goroutine without synchronization, which is a data race (will be reported under go test -race). Capture the received values via a channel, mutex-protected struct, or sync/atomic to make the test race-free.
| w.Header().Set("X-Upstream", "local-fake") | ||
| _, _ = w.Write([]byte("controlplane ok")) | ||
| })) |
There was a problem hiding this comment.
The fake upstream responds with a normal 200 OK body here, so this test doesn't exercise the real /ts2021 upgrade flow (101 Switching Protocols + hijack/tunneling). If the intent is to regression-test upgrade handling, consider making the upstream perform an actual 101 upgrade (via hijack) and asserting the proxy forwards the 101 and tunnels data.
| ProxyT depends on the frontend preserving Tailscale's `/ts2021` control-protocol upgrade request intact. Direct deployments, Tailscale Funnel, and traditional reverse proxies such as Nginx, Apache, and Caddy are the most reliable options. | ||
|
|
||
| Managed CDN and edge-proxy platforms that only support standard WebSocket `GET` handshakes, or that normalize non-standard upgrade traffic, are not compatible. In practice this means Cloudflare proxy/tunnel/workers are not supported, and platforms such as CloudFront, Fastly free tier, and Railway-style managed HTTP edges may fail depending on how their edge handles `POST` upgrades. |
There was a problem hiding this comment.
The README now refers to the project as "ProxyT" here, but nearby text still uses "Proxyt" to refer to the same thing. Please standardize the product name throughout this document (and reserve proxyt for the CLI command) to avoid confusing readers.
Covers the /ts2021 proxy-compatibility docs, added regression test, and supporting logs.
Signed-off-by: Lee Briggs lee@leebriggs.co.uk