Skip to content

Commit

Permalink
fix(httpext): return error on unsupported 101 response
Browse files Browse the repository at this point in the history
The hanging behavior described in #971 happens after this Golang change:
golang/go@5416204
, 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]: golang/go#31391

Closes #971
  • Loading branch information
Ivan Mirić committed Sep 24, 2019
1 parent 7963475 commit 97f1915
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/netext/httpext/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package httpext
import (
"bytes"
"context"
"fmt"
"io"
"io/ioutil"
"net"
Expand Down Expand Up @@ -287,6 +288,11 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
mreq := preq.Req.WithContext(ctx)
res, resErr := client.Do(mreq)

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 {
Expand Down
27 changes: 27 additions & 0 deletions lib/netext/httpext/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ import (
"io"
"io/ioutil"
"net/http"
"net/http/httptest"
"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"
)

Expand Down Expand Up @@ -81,4 +85,27 @@ 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")
})
}

0 comments on commit 97f1915

Please sign in to comment.