Skip to content

Commit

Permalink
fix(app): clone tls configuration for websocket dialer (#19423)
Browse files Browse the repository at this point in the history
  • Loading branch information
gabrielcorado authored Dec 19, 2022
1 parent 138db7f commit fa7ade2
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 16 deletions.
74 changes: 59 additions & 15 deletions lib/srv/app/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ type suiteConfig struct {
ServerStreamer events.Streamer
// ValidateRequest is a function that will validate the request received by the application.
ValidateRequest func(*Suite, *http.Request)
// UseWebsockets will make the application server use a websocket for connection.
UseWebsockets bool
// EnableHTTP2 defines if the test server will support HTTP2.
EnableHTTP2 bool
// CloudImporter will use the given cloud importer for the app server.
CloudImporter labels.Importer
// AppLabels are the labels assigned to the application.
Expand Down Expand Up @@ -200,7 +200,7 @@ func SetUpSuiteWithConfig(t *testing.T, config suiteConfig) *Suite {
s.message = uuid.New().String()

s.testhttp = httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if config.UseWebsockets {
if strings.ToLower(r.Header.Get("upgrade")) == "websocket" {
upgrader := websocket.Upgrader{
ReadBufferSize: 1024,
WriteBufferSize: 1024,
Expand All @@ -218,11 +218,16 @@ func SetUpSuiteWithConfig(t *testing.T, config suiteConfig) *Suite {
config.ValidateRequest(s, r)
}
}))
// Add NextProtos to support both protocols: h2, http/1.1
s.testhttp.Config.TLSConfig = &tls.Config{Time: s.clock.Now}
if config.UseWebsockets {
if config.EnableHTTP2 {
s.testhttp.EnableHTTP2 = true
// Add NextProtos to support both protocols: h2, http/1.1
s.testhttp.Config.TLSConfig.NextProtos = []string{"h2", "http/1.1"}
s.testhttp.StartTLS()
} else {
s.testhttp.Start()
}
s.testhttp.Start()

// Extract the hostport that the in-memory HTTP server is running on.
u, err := url.Parse(s.testhttp.URL)
Expand All @@ -241,9 +246,10 @@ func SetUpSuiteWithConfig(t *testing.T, config suiteConfig) *Suite {
Name: "foo",
Labels: appLabels,
}, types.AppSpecV3{
URI: s.testhttp.URL,
PublicAddr: "foo.example.com",
DynamicLabels: types.LabelsToV2(dynamicLabels),
URI: s.testhttp.URL,
PublicAddr: "foo.example.com",
InsecureSkipVerify: true,
DynamicLabels: types.LabelsToV2(dynamicLabels),
})
require.NoError(t, err)
appAWS, err := types.NewAppV3(types.Metadata{
Expand Down Expand Up @@ -369,8 +375,9 @@ func TestStart(t *testing.T) {
Name: "foo",
Labels: staticLabels,
}, types.AppSpecV3{
URI: s.testhttp.URL,
PublicAddr: "foo.example.com",
URI: s.testhttp.URL,
PublicAddr: "foo.example.com",
InsecureSkipVerify: true,
DynamicLabels: map[string]types.CommandLabelV2{
dynamicLabelName: {
Period: dynamicLabelPeriod,
Expand Down Expand Up @@ -598,7 +605,6 @@ func TestHandleConnectionWS(t *testing.T) {
// websocket transport header rewriter delegate.
require.Equal(t, s.serverPort, r.Header.Get(forward.XForwardedPort))
},
UseWebsockets: true,
})

s.checkWSResponse(t, s.clientCertificate, func(messageType int, message string) {
Expand All @@ -607,6 +613,42 @@ func TestHandleConnectionWS(t *testing.T) {
})
}

// TestHandleConnectionHTTP2WS given a server that supports HTTP2, make a
// request and then connect to WebSocket, ensuring that both succeed.
//
// This test guarantees the server is capable of handing requests and websockets
// in different HTTP versions.
func TestHandleConnectionHTTP2WS(t *testing.T) {
s := SetUpSuiteWithConfig(t, suiteConfig{
EnableHTTP2: true,
ValidateRequest: func(s *Suite, r *http.Request) {
// Differentiate WebSocket requests.
if strings.ToLower(r.Header.Get("upgrade")) == "websocket" {
// Expect WS requests to be using http 1.
require.Equal(t, 1, r.ProtoMajor)
return
}

// Expect http requests to be using h2.
require.Equal(t, 2, r.ProtoMajor)
},
})

// First, make the request. This will be using HTTP2.
s.checkHTTPResponse(t, s.clientCertificate, func(resp *http.Response) {
require.Equal(t, resp.StatusCode, http.StatusOK)
buf, err := io.ReadAll(resp.Body)
require.NoError(t, err)
require.Equal(t, strings.TrimSpace(string(buf)), s.message)
})

// Second, make the WebSocket connection. This will be using HTTP/1.1
s.checkWSResponse(t, s.clientCertificate, func(messageType int, message string) {
require.Equal(t, websocket.TextMessage, messageType)
require.Equal(t, s.message, message)
})
}

// TestAuthorize verifies that only authorized requests are handled.
func TestAuthorize(t *testing.T) {
tests := []struct {
Expand Down Expand Up @@ -873,8 +915,9 @@ func (s *Suite) checkHTTPResponse(t *testing.T, clientCert tls.Certificate, chec
checkResp(resp)
require.NoError(t, resp.Body.Close())

// Close should not trigger an error.
require.NoError(t, s.appServer.Close())
// Close should not trigger an error. Closing the connection is enough to
// get out of the HandleConnection routine.
require.NoError(t, pw.Close())

// Wait for the application server to actually stop serving before
// closing the test. This will make sure the server removes the listeners
Expand Down Expand Up @@ -929,8 +972,9 @@ func (s *Suite) checkWSResponse(t *testing.T, clientCert tls.Certificate, checkM
// This should not trigger an error.
require.NoError(t, ws.Close())

// Close should not trigger an error.
require.NoError(t, s.appServer.Close())
// Close should not trigger an error. Closing the connection is enough to
// get out of the HandleConnection routine.
require.NoError(t, pw.Close())

// Wait for the application server to actually stop serving before
// closing the test. This will make sure the server removes the listeners
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/app/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func newTransport(ctx context.Context, c *transportConfig) (*transport, error) {
c: c,
uri: uri,
tr: tr,
ws: newWebsocketTransport(uri, tr.TLSClientConfig, c),
ws: newWebsocketTransport(uri, tr.TLSClientConfig.Clone(), c),
}, nil
}

Expand Down

0 comments on commit fa7ade2

Please sign in to comment.