From ddae749111c7bd5f9a697933fc378d40aabbfe87 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 2 Nov 2015 22:34:43 -0500 Subject: [PATCH] Read error from failed upgrade attempts --- .../unversioned/remotecommand/remotecommand.go | 4 ---- pkg/util/httpstream/spdy/roundtripper.go | 2 +- pkg/util/httpstream/spdy/roundtripper_test.go | 16 +++++++++++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/pkg/client/unversioned/remotecommand/remotecommand.go b/pkg/client/unversioned/remotecommand/remotecommand.go index 4feb953d95f..f5ed51577ab 100644 --- a/pkg/client/unversioned/remotecommand/remotecommand.go +++ b/pkg/client/unversioned/remotecommand/remotecommand.go @@ -130,10 +130,6 @@ func (e *streamExecutor) Dial(protocols ...string) (httpstream.Connection, strin } defer resp.Body.Close() - if resp.StatusCode != http.StatusSwitchingProtocols { - return nil, "", fmt.Errorf("unexpected response status code %d (%s)", resp.StatusCode, http.StatusText(resp.StatusCode)) - } - conn, err := e.upgrader.NewConnection(resp) if err != nil { return nil, "", err diff --git a/pkg/util/httpstream/spdy/roundtripper.go b/pkg/util/httpstream/spdy/roundtripper.go index b3307936fa4..47b67307735 100644 --- a/pkg/util/httpstream/spdy/roundtripper.go +++ b/pkg/util/httpstream/spdy/roundtripper.go @@ -205,7 +205,7 @@ func (s *SpdyRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) func (s *SpdyRoundTripper) NewConnection(resp *http.Response) (httpstream.Connection, error) { connectionHeader := strings.ToLower(resp.Header.Get(httpstream.HeaderConnection)) upgradeHeader := strings.ToLower(resp.Header.Get(httpstream.HeaderUpgrade)) - if !strings.Contains(connectionHeader, strings.ToLower(httpstream.HeaderUpgrade)) || !strings.Contains(upgradeHeader, strings.ToLower(HeaderSpdy31)) { + if (resp.StatusCode != http.StatusSwitchingProtocols) || !strings.Contains(connectionHeader, strings.ToLower(httpstream.HeaderUpgrade)) || !strings.Contains(upgradeHeader, strings.ToLower(HeaderSpdy31)) { defer resp.Body.Close() responseError := "" responseErrorBytes, err := ioutil.ReadAll(resp.Body) diff --git a/pkg/util/httpstream/spdy/roundtripper_test.go b/pkg/util/httpstream/spdy/roundtripper_test.go index babd23c9011..7860431dab3 100644 --- a/pkg/util/httpstream/spdy/roundtripper_test.go +++ b/pkg/util/httpstream/spdy/roundtripper_test.go @@ -39,30 +39,42 @@ func TestRoundTripAndNewConnection(t *testing.T) { clientTLS *tls.Config serverConnectionHeader string serverUpgradeHeader string + serverStatusCode int shouldError bool }{ "no headers": { serverFunc: httptest.NewServer, serverConnectionHeader: "", serverUpgradeHeader: "", + serverStatusCode: http.StatusSwitchingProtocols, shouldError: true, }, "no upgrade header": { serverFunc: httptest.NewServer, serverConnectionHeader: "Upgrade", serverUpgradeHeader: "", + serverStatusCode: http.StatusSwitchingProtocols, shouldError: true, }, "no connection header": { serverFunc: httptest.NewServer, serverConnectionHeader: "", serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: true, + }, + "no switching protocol status code": { + serverFunc: httptest.NewServer, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusForbidden, shouldError: true, }, "http": { serverFunc: httptest.NewServer, serverConnectionHeader: "Upgrade", serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, shouldError: false, }, "https (invalid hostname + InsecureSkipVerify)": { @@ -81,6 +93,7 @@ func TestRoundTripAndNewConnection(t *testing.T) { clientTLS: &tls.Config{InsecureSkipVerify: true}, serverConnectionHeader: "Upgrade", serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, shouldError: false, }, "https (valid hostname + RootCAs)": { @@ -99,6 +112,7 @@ func TestRoundTripAndNewConnection(t *testing.T) { clientTLS: &tls.Config{RootCAs: localhostPool}, serverConnectionHeader: "Upgrade", serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, shouldError: false, }, } @@ -112,7 +126,7 @@ func TestRoundTripAndNewConnection(t *testing.T) { w.Header().Set(httpstream.HeaderConnection, testCase.serverConnectionHeader) w.Header().Set(httpstream.HeaderUpgrade, testCase.serverUpgradeHeader) - w.WriteHeader(http.StatusSwitchingProtocols) + w.WriteHeader(testCase.serverStatusCode) return }