Skip to content

Commit ae8abde

Browse files
rgarciaclaude
andcommitted
chromedriverproxy: handle comma-separated X-Forwarded-Proto
X-Forwarded-Proto can be a comma-separated list ("https, http") when a request traverses multiple proxies. clientWSScheme used the raw header value, so it wouldn't match "https" and fell back to ws:// — silently reintroducing the TLS-ingress bug this PR fixes. Take the first (client-facing) token before comparing. Adds a "https, http" case to the regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e4d51ec commit ae8abde

2 files changed

Lines changed: 36 additions & 23 deletions

File tree

server/lib/chromedriverproxy/proxy.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,13 @@ func handleCreateSession(w http.ResponseWriter, r *http.Request, logger *slog.Lo
190190
// there's no TLS indication (e.g. the docker plaintext path).
191191
func clientWSScheme(r *http.Request) string {
192192
if proto := r.Header.Get("X-Forwarded-Proto"); proto != "" {
193+
// X-Forwarded-Proto may be a comma-separated list when the request
194+
// traverses multiple proxies (e.g. "https, http"); the first value is
195+
// the original client-facing scheme.
196+
if i := strings.IndexByte(proto, ','); i >= 0 {
197+
proto = proto[:i]
198+
}
199+
proto = strings.TrimSpace(proto)
193200
if strings.EqualFold(proto, "https") || strings.EqualFold(proto, "wss") {
194201
return "wss"
195202
}

server/lib/chromedriverproxy/proxy_test.go

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -131,31 +131,37 @@ func TestHandler_PostSession_InjectsDebuggerAddress(t *testing.T) {
131131
// webSocketUrl is wss:// — a ws:// URL would be unreachable through the TLS
132132
// listener (this is what broke the BiDi tests on the hypeman :9224 ingress).
133133
func TestHandler_PostSession_WSSchemeFromForwardedProto(t *testing.T) {
134-
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
135-
w.Header().Set("Content-Type", "application/json")
136-
w.WriteHeader(http.StatusOK)
137-
w.Write([]byte(`{"value":{"sessionId":"abc123","capabilities":{"webSocketUrl":"ws://127.0.0.1:9225/session/abc123"}}}`))
138-
}))
139-
defer backend.Close()
140-
141-
backendURL, _ := url.Parse(backend.URL)
142-
handler := Handler(silentLogger(), testOptions(backendURL.Host, "127.0.0.1:9911"))
143-
144-
req := httptest.NewRequest(http.MethodPost, "/session", strings.NewReader(`{"capabilities":{}}`))
145-
req.Host = "inst.dev-yul-hypeman-1.kernel.sh:9224"
146-
req.Header.Set("Content-Type", "application/json")
147-
req.Header.Set("X-Forwarded-Proto", "https")
148-
rec := httptest.NewRecorder()
134+
// "https" plus the multi-proxy comma-separated form ("https, http"), where
135+
// only the first (client-facing) value should decide the scheme.
136+
for _, proto := range []string{"https", "https, http"} {
137+
t.Run(proto, func(t *testing.T) {
138+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
139+
w.Header().Set("Content-Type", "application/json")
140+
w.WriteHeader(http.StatusOK)
141+
w.Write([]byte(`{"value":{"sessionId":"abc123","capabilities":{"webSocketUrl":"ws://127.0.0.1:9225/session/abc123"}}}`))
142+
}))
143+
defer backend.Close()
144+
145+
backendURL, _ := url.Parse(backend.URL)
146+
handler := Handler(silentLogger(), testOptions(backendURL.Host, "127.0.0.1:9911"))
147+
148+
req := httptest.NewRequest(http.MethodPost, "/session", strings.NewReader(`{"capabilities":{}}`))
149+
req.Host = "inst.dev-yul-hypeman-1.kernel.sh:9224"
150+
req.Header.Set("Content-Type", "application/json")
151+
req.Header.Set("X-Forwarded-Proto", proto)
152+
rec := httptest.NewRecorder()
149153

150-
handler.ServeHTTP(rec, req)
151-
require.Equal(t, http.StatusOK, rec.Code)
154+
handler.ServeHTTP(rec, req)
155+
require.Equal(t, http.StatusOK, rec.Code)
152156

153-
var respBody map[string]interface{}
154-
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &respBody))
155-
value := respBody["value"].(map[string]interface{})
156-
respCaps := value["capabilities"].(map[string]interface{})
157-
assert.Equal(t, "wss://inst.dev-yul-hypeman-1.kernel.sh:9224/session/abc123", respCaps["webSocketUrl"],
158-
"webSocketUrl must be wss:// when X-Forwarded-Proto is https")
157+
var respBody map[string]interface{}
158+
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &respBody))
159+
value := respBody["value"].(map[string]interface{})
160+
respCaps := value["capabilities"].(map[string]interface{})
161+
assert.Equal(t, "wss://inst.dev-yul-hypeman-1.kernel.sh:9224/session/abc123", respCaps["webSocketUrl"],
162+
"webSocketUrl must be wss:// when X-Forwarded-Proto's first value is https (got %q)", proto)
163+
})
164+
}
159165
}
160166

161167
// TestHandler_RewritesHostAndStripsOrigin is a regression test for the

0 commit comments

Comments
 (0)