Skip to content

Commit

Permalink
Always set the correct content-length for requests
Browse files Browse the repository at this point in the history
This would restore the behavior pre-#989, so that any content-length header values manually specified by users would be overwritten by k6. Only, this time it won't be done silently, a warning would be shown every time there was a mismatch.

The HAR converter is also modified to not include the content-length header in the generated sources. While that header isn't going to be a problem if its values matches the actual body content length, some HAR files are broken and include the header without any body data, and other times users may modify the body but forget the header. So, since k6 would automatically send it, we shouldn't include it in the script.
  • Loading branch information
na-- committed Aug 7, 2019
1 parent 8e93258 commit 146489e
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 48 deletions.
12 changes: 4 additions & 8 deletions cmd/testdata/example.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions converter/har/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,13 @@ func buildK6RequestObject(req *Request) (string, error) {
func buildK6Headers(headers []Header) []string {
var h []string
if len(headers) > 0 {
m := make(map[string]Header)
ignored := map[string]bool{"cookie": true, "content-length": true}
for _, header := range headers {
name := strings.ToLower(header.Name)
_, exists := m[name]
// Avoid SPDY's, duplicated or cookie headers
if !exists && name[0] != ':' && name != "cookie" {
m[strings.ToLower(header.Name)] = header
_, isIgnored := ignored[name]
// Avoid SPDY's, duplicated or ignored headers
if !isIgnored && name[0] != ':' {
ignored[name] = true
h = append(h, fmt.Sprintf("%q: %q", header.Name, header.Value))
}
}
Expand Down
5 changes: 2 additions & 3 deletions js/modules/k6/http/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1395,9 +1395,8 @@ 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 211")
require.NoError(t, err)
// TODO: test for warnings with the log hook
})
})
}
Expand Down
58 changes: 26 additions & 32 deletions lib/netext/httpext/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ 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
Expand Down Expand Up @@ -346,47 +342,31 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
Headers: preq.Req.Header,
}

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 {

// 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()

switch {
case len(preq.Compressions) > 0:
var (
contentEncoding string
err error
)
preq.Body, contentEncoding, err = compressBody(preq.Compressions, ioutil.NopCloser(preq.Body))
if len(preq.Compressions) > 0 {
compressedBody, contentEncoding, err := compressBody(preq.Compressions, ioutil.NopCloser(preq.Body))
if err != nil {
return nil, err
}
preq.Body = compressedBody

if preq.Req.Header.Get("Content-Length") == "" {
preq.Req.ContentLength = int64(preq.Body.Len())
} else {
state.Logger.Warningf(compressionHeaderOverwriteMessage, "Content-Length", preq.Req.Method, preq.Req.URL)
}
if preq.Req.Header.Get("Content-Encoding") == "" {
currentContentEncoding := preq.Req.Header.Get("Content-Encoding")
if currentContentEncoding == "" {
preq.Req.Header.Set("Content-Encoding", contentEncoding)
} else {
state.Logger.Warningf(compressionHeaderOverwriteMessage, "Content-Encoding", preq.Req.Method, preq.Req.URL)
} else if currentContentEncoding != contentEncoding {
state.Logger.Warningf(
"There's a mismatch between the desired `compression` the manually set `Content-Encoding` header "+
"in the %s request for '%s', the custom header has precedence and won't be overwritten. "+
"This may result in invalid data being sent to the server.", preq.Req.Method, preq.Req.URL,
)
}
case preq.Req.Header.Get("Content-Length") == "":
preq.Req.ContentLength = int64(preq.Body.Len())
}
// 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

preq.Req.ContentLength = int64(preq.Body.Len()) // This will make Go set the content-length header
preq.Req.GetBody = func() (io.ReadCloser, error) {
// using `Bytes()` should reuse the same buffer and as such help with the memory usage. We
// should not be writing to it any way so there shouldn't be way to corrupt it (?)
Expand All @@ -396,6 +376,20 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
preq.Req.Body, _ = preq.Req.GetBody()
}

if contentLengthHeader := preq.Req.Header.Get("Content-Length"); contentLengthHeader != "" {
// The content-length header was set by the user, delete it (since Go
// will set it automatically) and warn if there were differences
preq.Req.Header.Del("Content-Length")
length, err := strconv.Atoi(contentLengthHeader)
if err != nil || preq.Req.ContentLength != int64(length) {
state.Logger.Warnf(
"The specified Content-Length header %q in the %s request for %s "+
"doesn't match the actual request body length of %d, so it will be ignored!",
contentLengthHeader, preq.Req.Method, preq.Req.URL, length,
)
}
}

tags := state.Options.RunTags.CloneTags()
for k, v := range preq.Tags {
tags[k] = v
Expand Down

0 comments on commit 146489e

Please sign in to comment.