From 12bd929b1e733918b517c40e2528ea485c238130 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ivan=20Miri=C4=87?= <ivan@loadimpact.com>
Date: Fri, 13 Sep 2019 10:22:00 +0200
Subject: [PATCH 1/2] fix(httpext): return error on unsupported 101 response

The hanging behavior described in #971 happens after this Golang change:
https://github.com/golang/go/commit/541620409dee210c5498cc38433dcf690f58f888
, released in 1.12. If a `101 Switching Protocols` response is received,
`response.Body` will be replaced by an underlying `net.Conn`, and
reading it blocks indefinitely without a protocol implementation.

This is not a big problem for k6 in this case, since we don't support
protocol upgrades with the current HTTP API as shown in the example, so
the agreed solution was to detect it and return an error.

This Golang change is currently incompatible with client timeouts[1],
though this will hopefully be resolved by the time we add support for
h2c (#970) or other protocols.

[1]: https://github.com/golang/go/issues/31391

Closes #971
---
 lib/netext/httpext/request.go      | 10 ++++++++++
 lib/netext/httpext/request_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go
index b9c5242b5e3..cc66b920330 100644
--- a/lib/netext/httpext/request.go
+++ b/lib/netext/httpext/request.go
@@ -23,6 +23,7 @@ package httpext
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
@@ -314,6 +315,15 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
 	mreq := preq.Req.WithContext(ctx)
 	res, resErr := client.Do(mreq)
 
+	// TODO(imiric): It would be safer to check for a writeable
+	// response body here instead of status code, but those are
+	// wrapped in a read-only body when using client timeouts and are
+	// unusable until https://github.com/golang/go/issues/31391 is fixed.
+	if res != nil && res.StatusCode == http.StatusSwitchingProtocols {
+		_ = res.Body.Close()
+		return nil, fmt.Errorf("unsupported response status: %s", res.Status)
+	}
+
 	resp.Body, resErr = readResponseBody(state, preq.ResponseType, res, resErr)
 	finishedReq := tracerTransport.processLastSavedRequest(wrapDecompressionError(resErr))
 	if finishedReq != nil {
diff --git a/lib/netext/httpext/request_test.go b/lib/netext/httpext/request_test.go
index 91ba642d0f3..ba00dca8230 100644
--- a/lib/netext/httpext/request_test.go
+++ b/lib/netext/httpext/request_test.go
@@ -6,10 +6,14 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"testing"
 
+	"github.com/loadimpact/k6/lib"
+	"github.com/loadimpact/k6/stats"
 	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -82,6 +86,29 @@ func TestMakeRequestError(t *testing.T) {
 		require.Error(t, err)
 		require.Equal(t, err.Error(), "unknown compressionType CompressionType(13)")
 	})
+
+	t.Run("invalid upgrade response", func(t *testing.T) {
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Add("Connection", "Upgrade")
+			w.Header().Add("Upgrade", "h2c")
+			w.WriteHeader(http.StatusSwitchingProtocols)
+		}))
+		defer srv.Close()
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		state := &lib.State{
+			Options:   lib.Options{RunTags: &stats.SampleTags{}},
+			Transport: srv.Client().Transport,
+		}
+		ctx = lib.WithState(ctx, state)
+		req, _ := http.NewRequest("GET", srv.URL, nil)
+		var preq = &ParsedHTTPRequest{Req: req, URL: &URL{u: req.URL}, Body: new(bytes.Buffer)}
+
+		res, err := MakeRequest(ctx, preq)
+
+		assert.Nil(t, res)
+		assert.EqualError(t, err, "unsupported response status: 101 Switching Protocols")
+	})
 }
 
 func TestURL(t *testing.T) {

From b3d9fd5109c01c801c47390d2baa71229547de27 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ivan=20Miri=C4=87?= <ivan@loadimpact.com>
Date: Fri, 27 Sep 2019 09:48:23 +0200
Subject: [PATCH 2/2] chore: disable godox in CI

See https://github.com/loadimpact/k6/pull/1172#discussion_r328727844
---
 .golangci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.golangci.yml b/.golangci.yml
index 361c5f6977f..62ff62cd901 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -45,4 +45,5 @@ linters:
   enable-all: true
   disable:
   - gochecknoinits
+  - godox
   fast: false