From 9efecb24498b6e1ad7bf678bbcf4abd247ec98e6 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 8 Apr 2019 13:07:40 +0300 Subject: [PATCH 01/23] Add ability to gzip compress request body fix #988 This adds `compression` param to all http methods with only current possible value `gzip` that will compress the body set the correct `Content-Encoding` and `Content-Length` and will send the request with the compressed body. --- js/modules/k6/http/request.go | 22 ++++++++++++++++ js/modules/k6/http/request_test.go | 42 ++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/js/modules/k6/http/request.go b/js/modules/k6/http/request.go index 36fcdab0eb8..6dff6a464bd 100644 --- a/js/modules/k6/http/request.go +++ b/js/modules/k6/http/request.go @@ -22,8 +22,10 @@ package http import ( "bytes" + "compress/gzip" "context" "fmt" + "io" "io/ioutil" "mime/multipart" "net/http" @@ -310,6 +312,26 @@ func (h *HTTP) parseRequest( case *HTTPCookieJar: result.ActiveJar = v.jar } + case "compression": + var algo = params.Get(k).ToString().String() + if algo != "gzip" { + return nil, fmt.Errorf("unknown compression algorithm %s, only supported algorithm is gzip", algo) + } + + var buf bytes.Buffer + var w = gzip.NewWriter(&buf) + var _, err = io.Copy(w, result.Req.Body) + if err != nil { + _ = w.Close() // just in case + return nil, err + } + if err = w.Close(); err != nil { + return nil, err + } + + result.Req.Header.Set("Content-Encoding", algo) + result.Req.Body = ioutil.NopCloser(&buf) + result.Req.ContentLength = int64(buf.Len()) case "redirects": result.Redirects = null.IntFrom(params.Get(k).ToInteger()) case "tags": diff --git a/js/modules/k6/http/request_test.go b/js/modules/k6/http/request_test.go index 39ebeb2b61e..2c980d1e231 100644 --- a/js/modules/k6/http/request_test.go +++ b/js/modules/k6/http/request_test.go @@ -22,8 +22,10 @@ package http import ( "bytes" + "compress/gzip" "context" "fmt" + "io" "io/ioutil" "net/http" "net/http/cookiejar" @@ -1212,6 +1214,46 @@ func TestSystemTags(t *testing.T) { } } +func TestRequestCompression(t *testing.T) { + t.Parallel() + tb, state, _, rt, _ := newRuntime(t) + defer tb.Cleanup() + + // We don't expect any failed requests + state.Options.Throw = null.BoolFrom(true) + + var text = ` + Lorem ipsum dolor sit amet, consectetur adipiscing elit. + Maecenas sed pharetra sapien. Nunc laoreet molestie ante ac gravida. + Etiam interdum dui viverra posuere egestas. Pellentesque at dolor tristique, + mattis turpis eget, commodo purus. Nunc orci aliquam.` + + tb.Mux.HandleFunc("/compressed-text", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, r.Header.Get("Content-Encoding"), "gzip") + + expectedLength, err := strconv.Atoi(r.Header.Get("Content-Length")) + require.NoError(t, err) + + var compressedBuf bytes.Buffer + n, err := io.Copy(&compressedBuf, r.Body) + require.Equal(t, int(n), expectedLength) + require.NoError(t, err) + + g, err := gzip.NewReader(&compressedBuf) + require.NoError(t, err) + + var buf bytes.Buffer + _, err = io.Copy(&buf, g) + require.NoError(t, err) + require.Equal(t, text, buf.String()) + })) + + _, err := common.RunString(rt, tb.Replacer.Replace(` + http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "gzip"}); + `)) + require.NoError(t, err) +} + func TestResponseTypes(t *testing.T) { t.Parallel() tb, state, _, rt, _ := newRuntime(t) From 9e435174d1dd1bfc9e3035ba1f3b95207ebd5b6e Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 8 Apr 2019 14:24:04 +0300 Subject: [PATCH 02/23] Add Release note for #988 --- release notes/upcoming.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/release notes/upcoming.md b/release notes/upcoming.md index 65f5d56e162..c71a70d24bc 100644 --- a/release notes/upcoming.md +++ b/release notes/upcoming.md @@ -2,11 +2,10 @@ TODO: Intro ## New Features! -### Category: Title (#533) +### HTTP: request body compression (#988) -Description of feature. +Now all http methods have an additional param called `compression`, with a single possible value `gzip`, which if set will send the body compressed as gzip. It will also correctly set both `Content-Encoding` and `Content-Length`. -**Docs**: [Title](http://k6.readme.io/docs/TODO) ## Bugs fixed! From d85f5281484b18fde4abe53195b5f41b1acc152a Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 8 Apr 2019 14:30:18 +0300 Subject: [PATCH 03/23] Add test for unsupported body compression --- js/modules/k6/http/request_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/js/modules/k6/http/request_test.go b/js/modules/k6/http/request_test.go index 2c980d1e231..3ad81dfe9bf 100644 --- a/js/modules/k6/http/request_test.go +++ b/js/modules/k6/http/request_test.go @@ -1248,10 +1248,21 @@ func TestRequestCompression(t *testing.T) { require.Equal(t, text, buf.String()) })) - _, err := common.RunString(rt, tb.Replacer.Replace(` + t.Run("gzip", func(t *testing.T) { + _, err := common.RunString(rt, tb.Replacer.Replace(` http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "gzip"}); `)) - require.NoError(t, err) + require.NoError(t, err) + }) + + t.Run("unsupported compression", func(t *testing.T) { + _, err := common.RunString(rt, tb.Replacer.Replace(` + http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "George"}); + `)) + require.Error(t, err) + require.Equal(t, err.Error(), + `GoError: unknown compression algorithm George, only supported algorithm is gzip`) + }) } func TestResponseTypes(t *testing.T) { From 39939381dcfc7f33394b545cd3b2367ffedc1fef Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 8 Apr 2019 14:52:00 +0300 Subject: [PATCH 04/23] Move closing of gzip in defer --- js/modules/k6/http/request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/modules/k6/http/request.go b/js/modules/k6/http/request.go index 6dff6a464bd..5dbc9941855 100644 --- a/js/modules/k6/http/request.go +++ b/js/modules/k6/http/request.go @@ -320,9 +320,9 @@ func (h *HTTP) parseRequest( var buf bytes.Buffer var w = gzip.NewWriter(&buf) + defer func() { _ = w.Close() }() var _, err = io.Copy(w, result.Req.Body) if err != nil { - _ = w.Close() // just in case return nil, err } if err = w.Close(); err != nil { From 679b66dbf28e7837bbc47debb459c658dd999d23 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 10 Apr 2019 17:05:50 +0300 Subject: [PATCH 05/23] Refactor body compression per PR comments - Move compression to httpext - support deflate and gzip - support combination of the two --- js/modules/k6/http/request.go | 32 ++++---- js/modules/k6/http/request_test.go | 91 +++++++++++++++++----- lib/netext/httpext/compression_type_gen.go | 79 +++++++++++++++++++ lib/netext/httpext/request.go | 58 ++++++++++++++ 4 files changed, 221 insertions(+), 39 deletions(-) create mode 100644 lib/netext/httpext/compression_type_gen.go diff --git a/js/modules/k6/http/request.go b/js/modules/k6/http/request.go index 5dbc9941855..c55a5d29fa9 100644 --- a/js/modules/k6/http/request.go +++ b/js/modules/k6/http/request.go @@ -22,10 +22,8 @@ package http import ( "bytes" - "compress/gzip" "context" "fmt" - "io" "io/ioutil" "mime/multipart" "net/http" @@ -313,25 +311,21 @@ func (h *HTTP) parseRequest( result.ActiveJar = v.jar } case "compression": - var algo = params.Get(k).ToString().String() - if algo != "gzip" { - return nil, fmt.Errorf("unknown compression algorithm %s, only supported algorithm is gzip", algo) - } - - var buf bytes.Buffer - var w = gzip.NewWriter(&buf) - defer func() { _ = w.Close() }() - var _, err = io.Copy(w, result.Req.Body) - if err != nil { - return nil, err + var algosString = strings.TrimSpace(params.Get(k).ToString().String()) + if algosString == "" { + continue } - if err = w.Close(); err != nil { - return nil, err + var algos = strings.Split(algosString, ",") + var err error + result.Compressions = make([]httpext.CompressionType, len(algos)) + for index, algo := range algos { + algo = strings.TrimSpace(algo) + result.Compressions[index], err = httpext.CompressionTypeString(algo) + if err != nil { + return nil, fmt.Errorf("unknown compression algorithm %s, supported algorithms are %s", + algo, httpext.CompressionTypeValues()) + } } - - result.Req.Header.Set("Content-Encoding", algo) - result.Req.Body = ioutil.NopCloser(&buf) - result.Req.ContentLength = int64(buf.Len()) case "redirects": result.Redirects = null.IntFrom(params.Get(k).ToInteger()) case "tags": diff --git a/js/modules/k6/http/request_test.go b/js/modules/k6/http/request_test.go index 3ad81dfe9bf..b174c563f47 100644 --- a/js/modules/k6/http/request_test.go +++ b/js/modules/k6/http/request_test.go @@ -22,6 +22,7 @@ package http import ( "bytes" + "compress/flate" "compress/gzip" "context" "fmt" @@ -1228,41 +1229,91 @@ func TestRequestCompression(t *testing.T) { Etiam interdum dui viverra posuere egestas. Pellentesque at dolor tristique, mattis turpis eget, commodo purus. Nunc orci aliquam.` + var decompress = func(algo string, input io.Reader) io.Reader { + switch algo { + case "gzip": + w, err := gzip.NewReader(input) + if err != nil { + t.Fatal(err) + } + return w + case "deflate": + return flate.NewReader(input) + default: + t.Fatal("unknown algorithm + " + algo) + } + return nil // unreachable + } + + var expectedEncoding string tb.Mux.HandleFunc("/compressed-text", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, r.Header.Get("Content-Encoding"), "gzip") + require.Equal(t, expectedEncoding, r.Header.Get("Content-Encoding")) expectedLength, err := strconv.Atoi(r.Header.Get("Content-Length")) require.NoError(t, err) - - var compressedBuf bytes.Buffer - n, err := io.Copy(&compressedBuf, r.Body) + var algos = strings.Split(expectedEncoding, ", ") + var compressedBuf = new(bytes.Buffer) + n, err := io.Copy(compressedBuf, r.Body) require.Equal(t, int(n), expectedLength) require.NoError(t, err) + var prev io.Reader = compressedBuf - g, err := gzip.NewReader(&compressedBuf) - require.NoError(t, err) + if expectedEncoding != "" { + for i := len(algos) - 1; i >= 0; i-- { + prev = decompress(algos[i], prev) + } + } var buf bytes.Buffer - _, err = io.Copy(&buf, g) + _, err = io.Copy(&buf, prev) require.NoError(t, err) require.Equal(t, text, buf.String()) })) - t.Run("gzip", func(t *testing.T) { - _, err := common.RunString(rt, tb.Replacer.Replace(` - http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "gzip"}); + var testCases = []struct { + name string + compression string + expectedError string + }{ + {compression: ""}, + {compression: " "}, + {compression: "gzip"}, + {compression: "gzip, gzip"}, + {compression: "gzip, gzip "}, + {compression: "gzip,gzip"}, + {compression: "gzip, gzip, gzip, gzip, gzip, gzip, gzip"}, + {compression: "deflate"}, + {compression: "deflate, gzip"}, + {compression: "gzip,deflate, gzip"}, + { + compression: "George", + expectedError: `unknown compression algorithm George`, + }, + { + compression: "gzip, George", + expectedError: `unknown compression algorithm George`, + }, + } + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.compression, func(t *testing.T) { + var algos = strings.Split(testCase.compression, ",") + for i, algo := range algos { + algos[i] = strings.TrimSpace(algo) + } + expectedEncoding = strings.Join(algos, ", ") + _, err := common.RunString(rt, tb.Replacer.Replace(` + http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "`+testCase.compression+`"}); `)) - require.NoError(t, err) - }) + if testCase.expectedError == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), testCase.expectedError) + } - t.Run("unsupported compression", func(t *testing.T) { - _, err := common.RunString(rt, tb.Replacer.Replace(` - http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "George"}); - `)) - require.Error(t, err) - require.Equal(t, err.Error(), - `GoError: unknown compression algorithm George, only supported algorithm is gzip`) - }) + }) + } } func TestResponseTypes(t *testing.T) { diff --git a/lib/netext/httpext/compression_type_gen.go b/lib/netext/httpext/compression_type_gen.go new file mode 100644 index 00000000000..63d0617f96c --- /dev/null +++ b/lib/netext/httpext/compression_type_gen.go @@ -0,0 +1,79 @@ +// Code generated by "enumer -type=CompressionType -transform=snake -json -text -trimprefix CompressionType -output compression_type_gen.go"; DO NOT EDIT. + +package httpext + +import ( + "encoding/json" + "fmt" +) + +const _CompressionTypeName = "gzipdeflate" + +var _CompressionTypeIndex = [...]uint8{0, 4, 11} + +func (i CompressionType) String() string { + if i >= CompressionType(len(_CompressionTypeIndex)-1) { + return fmt.Sprintf("CompressionType(%d)", i) + } + return _CompressionTypeName[_CompressionTypeIndex[i]:_CompressionTypeIndex[i+1]] +} + +var _CompressionTypeValues = []CompressionType{0, 1} + +var _CompressionTypeNameToValueMap = map[string]CompressionType{ + _CompressionTypeName[0:4]: 0, + _CompressionTypeName[4:11]: 1, +} + +// CompressionTypeString retrieves an enum value from the enum constants string name. +// Throws an error if the param is not part of the enum. +func CompressionTypeString(s string) (CompressionType, error) { + if val, ok := _CompressionTypeNameToValueMap[s]; ok { + return val, nil + } + return 0, fmt.Errorf("%s does not belong to CompressionType values", s) +} + +// CompressionTypeValues returns all values of the enum +func CompressionTypeValues() []CompressionType { + return _CompressionTypeValues +} + +// IsACompressionType returns "true" if the value is listed in the enum definition. "false" otherwise +func (i CompressionType) IsACompressionType() bool { + for _, v := range _CompressionTypeValues { + if i == v { + return true + } + } + return false +} + +// MarshalJSON implements the json.Marshaler interface for CompressionType +func (i CompressionType) MarshalJSON() ([]byte, error) { + return json.Marshal(i.String()) +} + +// UnmarshalJSON implements the json.Unmarshaler interface for CompressionType +func (i *CompressionType) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return fmt.Errorf("CompressionType should be a string, got %s", data) + } + + var err error + *i, err = CompressionTypeString(s) + return err +} + +// MarshalText implements the encoding.TextMarshaler interface for CompressionType +func (i CompressionType) MarshalText() ([]byte, error) { + return []byte(i.String()), nil +} + +// UnmarshalText implements the encoding.TextUnmarshaler interface for CompressionType +func (i *CompressionType) UnmarshalText(text []byte) error { + var err error + *i, err = CompressionTypeString(string(text)) + return err +} diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index e60ed72a01e..be066590bc9 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -22,6 +22,7 @@ package httpext import ( "bytes" + "compress/flate" "compress/gzip" "compress/zlib" "context" @@ -70,6 +71,22 @@ func (u URL) GetURL() *url.URL { return u.u } +// CompressionType is used to specify what compression is to be used to compress the body of a +// request +// The conversion and validation methods are auto-generated with https://github.com/alvaroloes/enumer: +//nolint: lll +//go:generate enumer -type=CompressionType -transform=snake -json -text -trimprefix CompressionType -output compression_type_gen.go +type CompressionType uint + +const ( + // CompressionTypeGzip compresses through gzip + CompressionTypeGzip CompressionType = iota + // CompressionTypeDeflate compresses through flate + CompressionTypeDeflate + // TODO: add compress(lzw), brotli maybe bzip2 and others listed at + // https://en.wikipedia.org/wiki/HTTP_compression#Content-Encoding_tokens +) + // Request represent an http request type Request struct { Method string `json:"method"` @@ -88,6 +105,7 @@ type ParsedHTTPRequest struct { Auth string Throw bool ResponseType ResponseType + Compressions []CompressionType Redirects null.Int ActiveJar *cookiejar.Jar Cookies map[string]*HTTPRequestCookie @@ -116,6 +134,46 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error Headers: preq.Req.Header, } if preq.Body != nil { + if len(preq.Compressions) > 0 { + var contentEncoding string + var prevBuf = preq.Req.Body + var buf *bytes.Buffer + for _, compressionType := range preq.Compressions { + if buf != nil { + prevBuf = ioutil.NopCloser(buf) + } + buf = new(bytes.Buffer) + + if contentEncoding != "" { + contentEncoding += ", " + } + contentEncoding += compressionType.String() + var w io.WriteCloser + switch compressionType { + case CompressionTypeGzip: + w = gzip.NewWriter(buf) + case CompressionTypeDeflate: + // Have the compression level as an option somewhere ? + // the error is only possible if we provide value outside of the range [-2, 9] + w, _ = flate.NewWriter(buf, flate.DefaultCompression) + default: + return nil, fmt.Errorf("unknown compressionType %s", compressionType) + } + defer func() { _ = w.Close() }() + var _, err = io.Copy(w, prevBuf) + if err != nil { + return nil, err + } + if err = w.Close(); err != nil { + return nil, err + } + } + + preq.Req.Body = ioutil.NopCloser(buf) + preq.Req.ContentLength = int64(buf.Len()) + preq.Req.Header.Set("Content-Encoding", contentEncoding) + } + // TODO: maybe hide this behind of flag in order for this to not happen for big post/puts? respReq.Body = preq.Body.String() } From 8479b0ca976e7d0d79bc5632525d0e915c5da946 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 10 Apr 2019 17:29:53 +0300 Subject: [PATCH 06/23] Update the release notes for #988 --- release notes/upcoming.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/release notes/upcoming.md b/release notes/upcoming.md index c71a70d24bc..6e1085a2a89 100644 --- a/release notes/upcoming.md +++ b/release notes/upcoming.md @@ -4,8 +4,7 @@ TODO: Intro ### HTTP: request body compression (#988) -Now all http methods have an additional param called `compression`, with a single possible value `gzip`, which if set will send the body compressed as gzip. It will also correctly set both `Content-Encoding` and `Content-Length`. - +Now all http methods have an additional param called `compression` that will make k6 to compress the body before sending it. It will also correctly set both `Content-Encoding` and `Content-Length`, except if they were not set previous to that. The current supported algorithms are `deflate` and `gzip` and any combination of the two separated by a comma (`,`). ## Bugs fixed! From 816ace7f1187d442d4c91d64b010947f95ca3651 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 10 Apr 2019 18:09:39 +0300 Subject: [PATCH 07/23] WIP on not overwriting user set Content-(Length|Encoding) headers --- js/modules/k6/http/request.go | 14 ++++++++++++- js/modules/k6/http/request_test.go | 32 +++++++++++++++++++++++++++--- lib/netext/httpext/request.go | 12 +++++++++-- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/js/modules/k6/http/request.go b/js/modules/k6/http/request.go index c55a5d29fa9..b03862be57a 100644 --- a/js/modules/k6/http/request.go +++ b/js/modules/k6/http/request.go @@ -30,6 +30,7 @@ import ( "net/textproto" "net/url" "reflect" + "strconv" "strings" "sync" "time" @@ -235,9 +236,20 @@ func (h *HTTP) parseRequest( } } + if contentLength := result.Req.Header.Get("Content-Length"); contentLength != "" { + length, err := strconv.Atoi(contentLength) + if err == nil { + result.Req.ContentLength = int64(length) + } + // maybe do something in the other case ... but no error + } if result.Body != nil { result.Req.Body = ioutil.NopCloser(result.Body) - result.Req.ContentLength = int64(result.Body.Len()) + if result.Req.Header.Get("Content-Length") == "" { + result.Req.ContentLength = int64(result.Body.Len()) + } else { + // TODO: print warning + } } if userAgent := state.Options.UserAgent; userAgent.String != "" { diff --git a/js/modules/k6/http/request_test.go b/js/modules/k6/http/request_test.go index b174c563f47..e90b4bd0cb0 100644 --- a/js/modules/k6/http/request_test.go +++ b/js/modules/k6/http/request_test.go @@ -1240,18 +1240,21 @@ func TestRequestCompression(t *testing.T) { case "deflate": return flate.NewReader(input) default: - t.Fatal("unknown algorithm + " + algo) + t.Fatal("unknown algorithm " + algo) } return nil // unreachable } - var expectedEncoding string + var ( + expectedEncoding string + actualEncoding string + ) tb.Mux.HandleFunc("/compressed-text", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.Equal(t, expectedEncoding, r.Header.Get("Content-Encoding")) expectedLength, err := strconv.Atoi(r.Header.Get("Content-Length")) require.NoError(t, err) - var algos = strings.Split(expectedEncoding, ", ") + var algos = strings.Split(actualEncoding, ", ") var compressedBuf = new(bytes.Buffer) n, err := io.Copy(compressedBuf, r.Body) require.Equal(t, int(n), expectedLength) @@ -1302,6 +1305,7 @@ func TestRequestCompression(t *testing.T) { algos[i] = strings.TrimSpace(algo) } expectedEncoding = strings.Join(algos, ", ") + actualEncoding = expectedEncoding _, err := common.RunString(rt, tb.Replacer.Replace(` http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "`+testCase.compression+`"}); `)) @@ -1314,6 +1318,28 @@ func TestRequestCompression(t *testing.T) { }) } + + t.Run("custom set header", func(t *testing.T) { + t.Run("encoding", func(t *testing.T) { + expectedEncoding = "not, valid" + actualEncoding = "gzip, deflate" + _, err := common.RunString(rt, tb.Replacer.Replace(` +http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "`+actualEncoding+`", headers: {"Content-Encoding": "`+expectedEncoding+`"}}); + `)) + require.NoError(t, err) + + }) + + t.Run("encoding and length", func(t *testing.T) { + expectedEncoding = "not, valid" + actualEncoding = "gzip, deflate" + _, err := common.RunString(rt, tb.Replacer.Replace(` +http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "`+actualEncoding+`", headers: {"Content-Encoding": "`+expectedEncoding+`", "Content-Length": "12"}}); + `)) + require.Error(t, err) + require.Contains(t, err.Error(), "http: ContentLength=261 with Body length 205") + }) + }) } func TestResponseTypes(t *testing.T) { diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index be066590bc9..4623c9b944f 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -170,8 +170,16 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error } preq.Req.Body = ioutil.NopCloser(buf) - preq.Req.ContentLength = int64(buf.Len()) - preq.Req.Header.Set("Content-Encoding", contentEncoding) + if preq.Req.Header.Get("Content-Length") == "" { + preq.Req.ContentLength = int64(buf.Len()) + } else { + // TODO: print warning + } + if preq.Req.Header.Get("Content-Encoding") == "" { + preq.Req.Header.Set("Content-Encoding", contentEncoding) + } else { + // TODO: print warning + } } // TODO: maybe hide this behind of flag in order for this to not happen for big post/puts? respReq.Body = preq.Body.String() From af39174daea6e1ed0a85c3ba89620d7c6811266a Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 08:34:23 +0300 Subject: [PATCH 08/23] Don't generate json/text marshalling for CompressionType --- lib/netext/httpext/compression_type_gen.go | 32 +--------------------- lib/netext/httpext/request.go | 2 +- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/lib/netext/httpext/compression_type_gen.go b/lib/netext/httpext/compression_type_gen.go index 63d0617f96c..c2ee00e79ce 100644 --- a/lib/netext/httpext/compression_type_gen.go +++ b/lib/netext/httpext/compression_type_gen.go @@ -1,9 +1,8 @@ -// Code generated by "enumer -type=CompressionType -transform=snake -json -text -trimprefix CompressionType -output compression_type_gen.go"; DO NOT EDIT. +// Code generated by "enumer -type=CompressionType -transform=snake -trimprefix CompressionType -output compression_type_gen.go"; DO NOT EDIT. package httpext import ( - "encoding/json" "fmt" ) @@ -48,32 +47,3 @@ func (i CompressionType) IsACompressionType() bool { } return false } - -// MarshalJSON implements the json.Marshaler interface for CompressionType -func (i CompressionType) MarshalJSON() ([]byte, error) { - return json.Marshal(i.String()) -} - -// UnmarshalJSON implements the json.Unmarshaler interface for CompressionType -func (i *CompressionType) UnmarshalJSON(data []byte) error { - var s string - if err := json.Unmarshal(data, &s); err != nil { - return fmt.Errorf("CompressionType should be a string, got %s", data) - } - - var err error - *i, err = CompressionTypeString(s) - return err -} - -// MarshalText implements the encoding.TextMarshaler interface for CompressionType -func (i CompressionType) MarshalText() ([]byte, error) { - return []byte(i.String()), nil -} - -// UnmarshalText implements the encoding.TextUnmarshaler interface for CompressionType -func (i *CompressionType) UnmarshalText(text []byte) error { - var err error - *i, err = CompressionTypeString(string(text)) - return err -} diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index 4623c9b944f..87e42f6b52d 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -75,7 +75,7 @@ func (u URL) GetURL() *url.URL { // request // The conversion and validation methods are auto-generated with https://github.com/alvaroloes/enumer: //nolint: lll -//go:generate enumer -type=CompressionType -transform=snake -json -text -trimprefix CompressionType -output compression_type_gen.go +//go:generate enumer -type=CompressionType -transform=snake -trimprefix CompressionType -output compression_type_gen.go type CompressionType uint const ( From afc72bed9b8ff0bf44105c39d3afe2d2114b53e8 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 08:42:00 +0300 Subject: [PATCH 09/23] WIP refactoring --- js/modules/k6/http/request_test.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/js/modules/k6/http/request_test.go b/js/modules/k6/http/request_test.go index e90b4bd0cb0..02657341c4a 100644 --- a/js/modules/k6/http/request_test.go +++ b/js/modules/k6/http/request_test.go @@ -1320,23 +1320,31 @@ func TestRequestCompression(t *testing.T) { } t.Run("custom set header", func(t *testing.T) { + expectedEncoding = "not, valid" + actualEncoding = "gzip, deflate" t.Run("encoding", func(t *testing.T) { - expectedEncoding = "not, valid" - actualEncoding = "gzip, deflate" _, err := common.RunString(rt, tb.Replacer.Replace(` -http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "`+actualEncoding+`", headers: {"Content-Encoding": "`+expectedEncoding+`"}}); - `)) + http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, + {"compression": "`+actualEncoding+`", + "headers": {"Content-Encoding": "`+expectedEncoding+`"} + } + ); + `)) require.NoError(t, err) }) t.Run("encoding and length", func(t *testing.T) { - expectedEncoding = "not, valid" - actualEncoding = "gzip, deflate" _, err := common.RunString(rt, tb.Replacer.Replace(` -http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, {"compression": "`+actualEncoding+`", headers: {"Content-Encoding": "`+expectedEncoding+`", "Content-Length": "12"}}); - `)) + http.post("HTTPBIN_URL/compressed-text", `+"`"+text+"`"+`, + {"compression": "`+actualEncoding+`", + "headers": {"Content-Encoding": "`+expectedEncoding+`", + "Content-Length": "12"} + } + ); + `)) require.Error(t, err) + // TODO: This probably shouldn't be like this require.Contains(t, err.Error(), "http: ContentLength=261 with Body length 205") }) }) From df03bd3b6d0de2a48be7555b513c77c826b99fa0 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 09:16:23 +0300 Subject: [PATCH 10/23] WIP on pritnting warning when reseting already set headers --- js/modules/k6/http/request.go | 33 +++++++++++++++--------------- js/modules/k6/http/request_test.go | 2 +- lib/netext/httpext/request.go | 6 ++++-- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/js/modules/k6/http/request.go b/js/modules/k6/http/request.go index b03862be57a..a96e1eec9ab 100644 --- a/js/modules/k6/http/request.go +++ b/js/modules/k6/http/request.go @@ -236,22 +236,6 @@ func (h *HTTP) parseRequest( } } - if contentLength := result.Req.Header.Get("Content-Length"); contentLength != "" { - length, err := strconv.Atoi(contentLength) - if err == nil { - result.Req.ContentLength = int64(length) - } - // maybe do something in the other case ... but no error - } - if result.Body != nil { - result.Req.Body = ioutil.NopCloser(result.Body) - if result.Req.Header.Get("Content-Length") == "" { - result.Req.ContentLength = int64(result.Body.Len()) - } else { - // TODO: print warning - } - } - if userAgent := state.Options.UserAgent; userAgent.String != "" { result.Req.Header.Set("User-Agent", userAgent.String) } @@ -368,6 +352,23 @@ func (h *HTTP) parseRequest( } } + if contentLength := result.Req.Header.Get("Content-Length"); contentLength != "" { + length, err := strconv.Atoi(contentLength) + if err == nil { + result.Req.ContentLength = int64(length) + } + // TODO: maybe do something in the other case ... but no error + } + if result.Body != nil { + result.Req.Body = ioutil.NopCloser(result.Body) + if result.Req.Header.Get("Content-Length") == "" { + result.Req.ContentLength = int64(result.Body.Len()) + } else { + // TODO: print line number, maybe don't print this at all ? + state.Logger.Warningf("Content-Length is specifically set won't reset it based on body length") + } + } + if result.ActiveJar != nil { httpext.SetRequestCookies(result.Req, result.ActiveJar, result.Cookies) } diff --git a/js/modules/k6/http/request_test.go b/js/modules/k6/http/request_test.go index 02657341c4a..e9bbe9249a5 100644 --- a/js/modules/k6/http/request_test.go +++ b/js/modules/k6/http/request_test.go @@ -1345,7 +1345,7 @@ func TestRequestCompression(t *testing.T) { `)) require.Error(t, err) // TODO: This probably shouldn't be like this - require.Contains(t, err.Error(), "http: ContentLength=261 with Body length 205") + require.Contains(t, err.Error(), "http: ContentLength=12 with Body length 205") }) }) } diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index 87e42f6b52d..ab947f031c7 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -173,12 +173,14 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error if preq.Req.Header.Get("Content-Length") == "" { preq.Req.ContentLength = int64(buf.Len()) } else { - // TODO: print warning + // TODO: print line + state.Logger.Warning("Content-Length is specifically set - won't be reset due to compression being specified") } if preq.Req.Header.Get("Content-Encoding") == "" { preq.Req.Header.Set("Content-Encoding", contentEncoding) } else { - // TODO: print warning + // TODO: print line + state.Logger.Warning("Content-Encoding is specifically set - won't be reset due to compression being specified") } } // TODO: maybe hide this behind of flag in order for this to not happen for big post/puts? From a13c01e11eabbc826e22f970ee0c7b56871fa059 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 10:54:12 +0300 Subject: [PATCH 11/23] Change flate to zlib for deflate support --- js/modules/k6/http/request_test.go | 10 +++++++--- lib/netext/httpext/request.go | 5 +---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/js/modules/k6/http/request_test.go b/js/modules/k6/http/request_test.go index e9bbe9249a5..da0d1f04304 100644 --- a/js/modules/k6/http/request_test.go +++ b/js/modules/k6/http/request_test.go @@ -22,8 +22,8 @@ package http import ( "bytes" - "compress/flate" "compress/gzip" + "compress/zlib" "context" "fmt" "io" @@ -1238,7 +1238,11 @@ func TestRequestCompression(t *testing.T) { } return w case "deflate": - return flate.NewReader(input) + w, err := zlib.NewReader(input) + if err != nil { + t.Fatal(err) + } + return w default: t.Fatal("unknown algorithm " + algo) } @@ -1345,7 +1349,7 @@ func TestRequestCompression(t *testing.T) { `)) require.Error(t, err) // TODO: This probably shouldn't be like this - require.Contains(t, err.Error(), "http: ContentLength=12 with Body length 205") + require.Contains(t, err.Error(), "http: ContentLength=12 with Body length 211") }) }) } diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index ab947f031c7..79d373cdcf4 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -22,7 +22,6 @@ package httpext import ( "bytes" - "compress/flate" "compress/gzip" "compress/zlib" "context" @@ -153,9 +152,7 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error case CompressionTypeGzip: w = gzip.NewWriter(buf) case CompressionTypeDeflate: - // Have the compression level as an option somewhere ? - // the error is only possible if we provide value outside of the range [-2, 9] - w, _ = flate.NewWriter(buf, flate.DefaultCompression) + w = zlib.NewWriter(buf) default: return nil, fmt.Errorf("unknown compressionType %s", compressionType) } From 8561e39a4e13bd69c0e64becea86f3a2d147145c Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 10:56:12 +0300 Subject: [PATCH 12/23] Update release notes --- release notes/upcoming.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release notes/upcoming.md b/release notes/upcoming.md index 6e1085a2a89..9e9e927ca8d 100644 --- a/release notes/upcoming.md +++ b/release notes/upcoming.md @@ -4,7 +4,7 @@ TODO: Intro ### HTTP: request body compression (#988) -Now all http methods have an additional param called `compression` that will make k6 to compress the body before sending it. It will also correctly set both `Content-Encoding` and `Content-Length`, except if they were not set previous to that. The current supported algorithms are `deflate` and `gzip` and any combination of the two separated by a comma (`,`). +Now all http methods have an additional param called `compression` that will make k6 compress the body before sending it. It will also correctly set both `Content-Encoding` and `Content-Length`, unless they were manually set in the request `headers` by the user. The current supported algorithms are `deflate` and `gzip` and any combination of the two separated by a comma (`,`). ## Bugs fixed! From eae1eae3e8da545b55a1902c76026df715704817 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 11:21:31 +0300 Subject: [PATCH 13/23] refactor the compression of body loggic to a function --- lib/netext/httpext/request.go | 75 ++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index 79d373cdcf4..29cfb00f98f 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -120,6 +120,43 @@ func stdCookiesToHTTPRequestCookies(cookies []*http.Cookie) map[string][]*HTTPRe return result } +func compressBody(algos []CompressionType, body io.Reader) (io.Reader, int64, string, error) { + var contentEncoding string + var prevBuf = body + var buf *bytes.Buffer + for _, compressionType := range algos { + if buf != nil { + prevBuf = buf + } + buf = new(bytes.Buffer) + + if contentEncoding != "" { + contentEncoding += ", " + } + contentEncoding += compressionType.String() + var w io.WriteCloser + switch compressionType { + case CompressionTypeGzip: + w = gzip.NewWriter(buf) + case CompressionTypeDeflate: + w = zlib.NewWriter(buf) + default: + return nil, 0, "", fmt.Errorf("unknown compressionType %s", compressionType) + } + // we don;t close in defer because zlib will write it's checksum again if it closes twice :( + var _, err = io.Copy(w, prevBuf) + if err != nil { + _ = w.Close() + return nil, 0, "", err + } + + if err = w.Close(); err != nil { + return nil, 0, "", err + } + } + return buf, int64(buf.Len()), contentEncoding, nil +} + // MakeRequest makes http request for tor the provided ParsedHTTPRequest //TODO break this function up //nolint: gocyclo @@ -134,41 +171,13 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error } if preq.Body != nil { if len(preq.Compressions) > 0 { - var contentEncoding string - var prevBuf = preq.Req.Body - var buf *bytes.Buffer - for _, compressionType := range preq.Compressions { - if buf != nil { - prevBuf = ioutil.NopCloser(buf) - } - buf = new(bytes.Buffer) - - if contentEncoding != "" { - contentEncoding += ", " - } - contentEncoding += compressionType.String() - var w io.WriteCloser - switch compressionType { - case CompressionTypeGzip: - w = gzip.NewWriter(buf) - case CompressionTypeDeflate: - w = zlib.NewWriter(buf) - default: - return nil, fmt.Errorf("unknown compressionType %s", compressionType) - } - defer func() { _ = w.Close() }() - var _, err = io.Copy(w, prevBuf) - if err != nil { - return nil, err - } - if err = w.Close(); err != nil { - return nil, err - } + compressedBody, length, contentEncoding, err := compressBody(preq.Compressions, preq.Req.Body) + if err != nil { + return nil, err } - - preq.Req.Body = ioutil.NopCloser(buf) + preq.Req.Body = ioutil.NopCloser(compressedBody) if preq.Req.Header.Get("Content-Length") == "" { - preq.Req.ContentLength = int64(buf.Len()) + preq.Req.ContentLength = length } else { // TODO: print line state.Logger.Warning("Content-Length is specifically set - won't be reset due to compression being specified") From ab32def5a1614f3a606bf6a0edd48f918785e2ba Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 16:41:21 +0300 Subject: [PATCH 14/23] Add more tests for handling rare/imposible body read problems --- lib/netext/httpext/request.go | 9 +++- lib/netext/httpext/request_test.go | 75 ++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 lib/netext/httpext/request_test.go diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index 29cfb00f98f..165c248e7bf 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -169,12 +169,16 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error Cookies: stdCookiesToHTTPRequestCookies(preq.Req.Cookies()), Headers: preq.Req.Header, } - if preq.Body != nil { + if preq.Req.Body != nil { if len(preq.Compressions) > 0 { compressedBody, length, contentEncoding, err := compressBody(preq.Compressions, preq.Req.Body) if err != nil { return nil, err } + if err = preq.Req.Body.Close(); err != nil { + return nil, err + } + preq.Req.Body = ioutil.NopCloser(compressedBody) if preq.Req.Header.Get("Content-Length") == "" { preq.Req.ContentLength = length @@ -189,6 +193,9 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error state.Logger.Warning("Content-Encoding is specifically set - won't be reset due to compression being specified") } } + } + + if preq.Body != nil { // TODO: maybe hide this behind of flag in order for this to not happen for big post/puts? respReq.Body = preq.Body.String() } diff --git a/lib/netext/httpext/request_test.go b/lib/netext/httpext/request_test.go new file mode 100644 index 00000000000..665b929d374 --- /dev/null +++ b/lib/netext/httpext/request_test.go @@ -0,0 +1,75 @@ +package httpext + +import ( + "context" + "io" + "net/http" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/require" +) + +type reader func([]byte) (int, error) + +func (r reader) Read(a []byte) (int, error) { + return ((func([]byte) (int, error))(r))(a) +} + +const badReadMsg = "bad read error for test" +const badCloseMsg = "bad close error for test" + +func badReadBody() io.Reader { + return reader(func(_ []byte) (int, error) { + return 0, errors.New(badReadMsg) + }) +} + +type closer func() error + +func (c closer) Close() error { + return ((func() error)(c))() +} + +func badCloseBody() io.ReadCloser { + return struct { + io.Reader + io.Closer + }{ + Reader: reader(func(_ []byte) (int, error) { + return 0, io.EOF + }), + Closer: closer(func() error { + return errors.New(badCloseMsg) + }), + } +} + +func TestMakeRequestError(t *testing.T) { + var ctx, cancel = context.WithCancel(context.Background()) + defer cancel() + + t.Run("bad read body", func(t *testing.T) { + req, err := http.NewRequest("GET", "https://wont.be.used", badReadBody()) + require.NoError(t, err) + var preq = &ParsedHTTPRequest{ + Req: req, + Compressions: []CompressionType{CompressionTypeGzip}, + } + _, err = MakeRequest(ctx, preq) + require.Error(t, err) + require.Equal(t, err.Error(), badReadMsg) + }) + + t.Run("bad close body", func(t *testing.T) { + req, err := http.NewRequest("GET", "https://wont.be.used", badCloseBody()) + require.NoError(t, err) + var preq = &ParsedHTTPRequest{ + Req: req, + Compressions: []CompressionType{CompressionTypeGzip}, + } + _, err = MakeRequest(ctx, preq) + require.Error(t, err) + require.Equal(t, err.Error(), badCloseMsg) + }) +} From a8388a0cc39153ccaf0a6ef6b026ea74fde3522f Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 17:13:40 +0300 Subject: [PATCH 15/23] Better error message when not overwriting headers and compressing the body --- lib/netext/httpext/request.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index 165c248e7bf..23571e55627 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -45,6 +45,10 @@ import ( null "gopkg.in/guregu/null.v3" ) +const compressionHeaderOverwriteMessage = "Both compression and the `%s` header were specified " + + "in the %s request for '%s', the custom header has precedence and won't be overwritten. " + + "This will likely result in invalid data being sent to the server." + // HTTPRequestCookie is a representation of a cookie used for request objects type HTTPRequestCookie struct { Name, Value string @@ -183,14 +187,12 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error if preq.Req.Header.Get("Content-Length") == "" { preq.Req.ContentLength = length } else { - // TODO: print line - state.Logger.Warning("Content-Length is specifically set - won't be reset due to compression being specified") + state.Logger.Warningf(compressionHeaderOverwriteMessage, "Content-Length", preq.Req.Method, preq.Req.URL) } if preq.Req.Header.Get("Content-Encoding") == "" { preq.Req.Header.Set("Content-Encoding", contentEncoding) } else { - // TODO: print line - state.Logger.Warning("Content-Encoding is specifically set - won't be reset due to compression being specified") + state.Logger.Warningf(compressionHeaderOverwriteMessage, "Content-Encoding", preq.Req.Method, preq.Req.URL) } } } From cca9c552bde91e7f069582e1736f17fd810ae33a Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 17:16:23 +0300 Subject: [PATCH 16/23] Add test for bad/unknown compression algorithm --- lib/netext/httpext/request_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/netext/httpext/request_test.go b/lib/netext/httpext/request_test.go index 665b929d374..c7878580c5b 100644 --- a/lib/netext/httpext/request_test.go +++ b/lib/netext/httpext/request_test.go @@ -1,6 +1,7 @@ package httpext import ( + "bytes" "context" "io" "net/http" @@ -72,4 +73,16 @@ func TestMakeRequestError(t *testing.T) { require.Error(t, err) require.Equal(t, err.Error(), badCloseMsg) }) + + t.Run("bad compression algorithm body", func(t *testing.T) { + req, err := http.NewRequest("GET", "https://wont.be.used", new(bytes.Buffer)) + require.NoError(t, err) + var preq = &ParsedHTTPRequest{ + Req: req, + Compressions: []CompressionType{CompressionType(13)}, + } + _, err = MakeRequest(ctx, preq) + require.Error(t, err) + require.Equal(t, err.Error(), "unknown compressionType CompressionType(13)") + }) } From 5257fa09a923ef6b308ebfa0d3813b2d52f72897 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 17:51:22 +0300 Subject: [PATCH 17/23] Refactor the whole setting of body and content length to MakeRequest from ParseRequest --- js/modules/k6/http/request.go | 19 ---------------- lib/netext/httpext/request.go | 36 ++++++++++++++++++++---------- lib/netext/httpext/request_test.go | 31 ++++++++++--------------- 3 files changed, 36 insertions(+), 50 deletions(-) diff --git a/js/modules/k6/http/request.go b/js/modules/k6/http/request.go index a96e1eec9ab..d448c3a0118 100644 --- a/js/modules/k6/http/request.go +++ b/js/modules/k6/http/request.go @@ -24,13 +24,11 @@ import ( "bytes" "context" "fmt" - "io/ioutil" "mime/multipart" "net/http" "net/textproto" "net/url" "reflect" - "strconv" "strings" "sync" "time" @@ -352,23 +350,6 @@ func (h *HTTP) parseRequest( } } - if contentLength := result.Req.Header.Get("Content-Length"); contentLength != "" { - length, err := strconv.Atoi(contentLength) - if err == nil { - result.Req.ContentLength = int64(length) - } - // TODO: maybe do something in the other case ... but no error - } - if result.Body != nil { - result.Req.Body = ioutil.NopCloser(result.Body) - if result.Req.Header.Get("Content-Length") == "" { - result.Req.ContentLength = int64(result.Body.Len()) - } else { - // TODO: print line number, maybe don't print this at all ? - state.Logger.Warningf("Content-Length is specifically set won't reset it based on body length") - } - } - if result.ActiveJar != nil { httpext.SetRequestCookies(result.Req, result.ActiveJar, result.Cookies) } diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index 23571e55627..bee60b5d38e 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -124,9 +124,9 @@ func stdCookiesToHTTPRequestCookies(cookies []*http.Cookie) map[string][]*HTTPRe return result } -func compressBody(algos []CompressionType, body io.Reader) (io.Reader, int64, string, error) { +func compressBody(algos []CompressionType, body io.ReadCloser) (io.Reader, int64, string, error) { var contentEncoding string - var prevBuf = body + var prevBuf io.Reader = body var buf *bytes.Buffer for _, compressionType := range algos { if buf != nil { @@ -158,7 +158,8 @@ func compressBody(algos []CompressionType, body io.Reader) (io.Reader, int64, st return nil, 0, "", err } } - return buf, int64(buf.Len()), contentEncoding, nil + + return buf, int64(buf.Len()), contentEncoding, body.Close() } // MakeRequest makes http request for tor the provided ParsedHTTPRequest @@ -173,15 +174,27 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error Cookies: stdCookiesToHTTPRequestCookies(preq.Req.Cookies()), Headers: preq.Req.Header, } - if preq.Req.Body != nil { + + if contentLength := preq.Req.Header.Get("Content-Length"); contentLength != "" { + length, err := strconv.Atoi(contentLength) + if err == nil { + preq.Req.ContentLength = int64(length) + } + // TODO: maybe do something in the other case ... but no error + } + + if preq.Body != nil { + preq.Req.Body = ioutil.NopCloser(preq.Body) + + // TODO: maybe hide this behind of flag in order for this to not happen for big post/puts? + // should we set this after the compression? what will be the point ? + respReq.Body = preq.Body.String() + if len(preq.Compressions) > 0 { compressedBody, length, contentEncoding, err := compressBody(preq.Compressions, preq.Req.Body) if err != nil { return nil, err } - if err = preq.Req.Body.Close(); err != nil { - return nil, err - } preq.Req.Body = ioutil.NopCloser(compressedBody) if preq.Req.Header.Get("Content-Length") == "" { @@ -194,14 +207,13 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error } else { state.Logger.Warningf(compressionHeaderOverwriteMessage, "Content-Encoding", preq.Req.Method, preq.Req.URL) } + } else if preq.Req.Header.Get("Content-Length") == "" { + preq.Req.ContentLength = int64(preq.Body.Len()) + } else { + state.Logger.Warningf("Content-Length is specifically set won't reset it based on body length") } } - if preq.Body != nil { - // TODO: maybe hide this behind of flag in order for this to not happen for big post/puts? - respReq.Body = preq.Body.String() - } - tags := state.Options.RunTags.CloneTags() for k, v := range preq.Tags { tags[k] = v diff --git a/lib/netext/httpext/request_test.go b/lib/netext/httpext/request_test.go index c7878580c5b..b28daafffbb 100644 --- a/lib/netext/httpext/request_test.go +++ b/lib/netext/httpext/request_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "io" + "io/ioutil" "net/http" "testing" @@ -46,39 +47,31 @@ func badCloseBody() io.ReadCloser { } } -func TestMakeRequestError(t *testing.T) { - var ctx, cancel = context.WithCancel(context.Background()) - defer cancel() - +func TestCompressionBodyError(t *testing.T) { + var algos = []CompressionType{CompressionTypeGzip} t.Run("bad read body", func(t *testing.T) { - req, err := http.NewRequest("GET", "https://wont.be.used", badReadBody()) - require.NoError(t, err) - var preq = &ParsedHTTPRequest{ - Req: req, - Compressions: []CompressionType{CompressionTypeGzip}, - } - _, err = MakeRequest(ctx, preq) + _, _, _, err := compressBody(algos, ioutil.NopCloser(badReadBody())) require.Error(t, err) require.Equal(t, err.Error(), badReadMsg) }) t.Run("bad close body", func(t *testing.T) { - req, err := http.NewRequest("GET", "https://wont.be.used", badCloseBody()) - require.NoError(t, err) - var preq = &ParsedHTTPRequest{ - Req: req, - Compressions: []CompressionType{CompressionTypeGzip}, - } - _, err = MakeRequest(ctx, preq) + _, _, _, err := compressBody(algos, badCloseBody()) require.Error(t, err) require.Equal(t, err.Error(), badCloseMsg) }) +} + +func TestMakeRequestError(t *testing.T) { + var ctx, cancel = context.WithCancel(context.Background()) + defer cancel() t.Run("bad compression algorithm body", func(t *testing.T) { - req, err := http.NewRequest("GET", "https://wont.be.used", new(bytes.Buffer)) + req, err := http.NewRequest("GET", "https://wont.be.used", nil) require.NoError(t, err) var preq = &ParsedHTTPRequest{ Req: req, + Body: new(bytes.Buffer), Compressions: []CompressionType{CompressionType(13)}, } _, err = MakeRequest(ctx, preq) From a1195f6eecad3d475b03d32a9abab28f67ccc48c Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 18:06:03 +0300 Subject: [PATCH 18/23] Test that badCompressionType is bad --- lib/netext/httpext/request_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/netext/httpext/request_test.go b/lib/netext/httpext/request_test.go index b28daafffbb..b0a5fde8682 100644 --- a/lib/netext/httpext/request_test.go +++ b/lib/netext/httpext/request_test.go @@ -67,12 +67,15 @@ func TestMakeRequestError(t *testing.T) { defer cancel() t.Run("bad compression algorithm body", func(t *testing.T) { - req, err := http.NewRequest("GET", "https://wont.be.used", nil) + var req, err = http.NewRequest("GET", "https://wont.be.used", nil) + require.NoError(t, err) + var badCompressionType = CompressionType(13) + require.False(t, badCompressionType.IsACompressionType()) var preq = &ParsedHTTPRequest{ Req: req, Body: new(bytes.Buffer), - Compressions: []CompressionType{CompressionType(13)}, + Compressions: []CompressionType{badCompressionType}, } _, err = MakeRequest(ctx, preq) require.Error(t, err) From 4fd808c511a3f14e2a021865784ee4eb3dac5940 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 11 Apr 2019 18:14:40 +0300 Subject: [PATCH 19/23] switch to switch --- lib/netext/httpext/request.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index bee60b5d38e..cc5c27ef23f 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -190,7 +190,8 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error // should we set this after the compression? what will be the point ? respReq.Body = preq.Body.String() - if len(preq.Compressions) > 0 { + switch { + case len(preq.Compressions) > 0: compressedBody, length, contentEncoding, err := compressBody(preq.Compressions, preq.Req.Body) if err != nil { return nil, err @@ -207,9 +208,9 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error } else { state.Logger.Warningf(compressionHeaderOverwriteMessage, "Content-Encoding", preq.Req.Method, preq.Req.URL) } - } else if preq.Req.Header.Get("Content-Length") == "" { + case preq.Req.Header.Get("Content-Length") == "": preq.Req.ContentLength = int64(preq.Body.Len()) - } else { + default: state.Logger.Warningf("Content-Length is specifically set won't reset it based on body length") } } From e4bca5033e4974c78e094032273ea5c5a88d3977 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 12 Apr 2019 10:26:38 +0300 Subject: [PATCH 20/23] small type --- lib/netext/httpext/request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index cc5c27ef23f..dc388a55cec 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -147,7 +147,7 @@ func compressBody(algos []CompressionType, body io.ReadCloser) (io.Reader, int64 default: return nil, 0, "", fmt.Errorf("unknown compressionType %s", compressionType) } - // we don;t close in defer because zlib will write it's checksum again if it closes twice :( + // we don't close in defer because zlib will write it's checksum again if it closes twice :( var _, err = io.Copy(w, prevBuf) if err != nil { _ = w.Close() From 727a31c94f75ffe23be5309801834c11d767f98d Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 12 Apr 2019 10:28:39 +0300 Subject: [PATCH 21/23] Remove warning when Content-Length is set for now --- lib/netext/httpext/request.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index dc388a55cec..084c234a1e4 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -210,9 +210,9 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error } case preq.Req.Header.Get("Content-Length") == "": preq.Req.ContentLength = int64(preq.Body.Len()) - default: - state.Logger.Warningf("Content-Length is specifically set won't reset it based on body length") } + // TODO: print some message in case we have Content-Length set so that we can warn users + // that setting it manually can lead to bad requests } tags := state.Options.RunTags.CloneTags() From 1f2604b4c41ab7b398123c164b73b0df2728d407 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 12 Apr 2019 10:55:58 +0300 Subject: [PATCH 22/23] Use the new CompressionType for uncompression matching as well --- lib/netext/httpext/request.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index 084c234a1e4..b2dd1e60816 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -339,11 +339,19 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error resp.Error = tracerTransport.errorMsg resp.ErrorCode = int(tracerTransport.errorCode) if resErr == nil && res != nil { - switch res.Header.Get("Content-Encoding") { - case "deflate": - res.Body, resErr = zlib.NewReader(res.Body) - case "gzip": - res.Body, resErr = gzip.NewReader(res.Body) + compression, err := CompressionTypeString(strings.TrimSpace(res.Header.Get("Content-Encoding"))) + if err == nil { // in case of error we just won't uncompress + switch compression { + case CompressionTypeDeflate: + res.Body, resErr = zlib.NewReader(res.Body) + case CompressionTypeGzip: + res.Body, resErr = gzip.NewReader(res.Body) + default: + // We have not implemented a compression ... :( + resErr = fmt.Errorf( + "Unsupported compressionType %s for uncompression. This is a bug in k6 please report it", + compression) + } } } if resErr == nil && res != nil { From 5ae8d5c8b6173792a38b1c2f416d78ef6728d1b9 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 12 Apr 2019 10:58:47 +0300 Subject: [PATCH 23/23] error strings should not be capitalized --- lib/netext/httpext/request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index b2dd1e60816..5e1d76d7549 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -349,7 +349,7 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error default: // We have not implemented a compression ... :( resErr = fmt.Errorf( - "Unsupported compressionType %s for uncompression. This is a bug in k6 please report it", + "unsupported compressionType %s for uncompression. This is a bug in k6 please report it", compression) } }