Open meiamsome opened 4 years ago
Ah... I think this is something that the Go net/http
stdlib helpfully "fixes" for us :man_facepalming: The good news is that it seems circumvent-able: https://stackoverflow.com/questions/24945806/add-unaltered-lowercase-headers-to-golang-http-request
Thanks for reporting this issue!
Ah... I think this is something that the Go
net/http
stdlib helpfully "fixes" for us 🤦♂ The good news is that it seems circumvent-able: https://stackoverflow.com/questions/24945806/add-unaltered-lowercase-headers-to-golang-http-requestThanks for reporting this issue!
IIRC, HTTP headers are case-insensitive, so if the server strictly checks for lower case only, it's non-compliant.
Yeah, that's why I put the evaluation needed
label. After all, if we have to make a lot of changes to k6 in order to support non-compliant HTTP servers, it's probably not a good idea to do so... But if we can make a few tweaks so that we don't automatically transform user-supplied custom request header, it's probably worth doing.
@meiamsome, can you give us some details about your use case?
I took a brief look at this, and while the fix itself is straightforward, adding tests for it seems impossible. See 3e3c3476.
The headers in the request passed to the handler are already transformed to the canonical case used in Go, so we can't write them out in their original case in the response to assert on it. Same goes for the internal httplib endpoints, of course.
The fix works though (confirmed with --http-debug
), but not sure if we should keep it if we can't ensure it will continue to work.
Another instance of this problem: https://community.k6.io/t/case-sensitive-headers/2745
The "fix" seems simple, so maybe we should implement it, even if we can't test it? :man_shrugging: And, if we really want to write a test, we can spin up a raw TCP server and parse the raw HTTP request without net/http
? That should be simple enough to implement?
This is not as straightforward as it seems on the surface, so I removed the milestone again... For context, https://github.com/golang/go/issues/5022#issuecomment-66225169 is the very good rationale for why Go doesn't have case-sensitive headers. And that makes complete sense, especially for HTTP servers. However, k6 is a testing tool and there's an argument to be made that artificial restrictions have no place in it. Sending arbitrary case in the HTTP header names is not against the RFC, the RFC just means that servers shouldn't care what the case of incoming header names is... And I'd argue that, even if it was against the RFC, it should be reasonably possible for k6 to even send somewhat malformed HTTP requests. Things like that are a normal part of QA and testing and a testing tool should reasonably accommodate them... :man_shrugging:
However, while it's seems like the "fix" on our side is simple, just replace result.Req.Header.Set(key, str)
with result.Req.Header[key] = []string{str}
, that would have implications we might not realize. For example, this will now produce a different result:
http.get(whatever, {headers: {
'foo': 'bar',
'Foo': 'baz',
}});
It's not clear that it will actually be a bad result, per se, since it will actually allow k6 to send multiple headers with the same header name. We currently can't do that (because we use a JS {key: value}
object and pass keys through CanonicalMIMEHeaderKey
), but the HTTP RFC actually allows it in some cases, see this from https://www.rfc-editor.org/rfc/rfc7230#section-3.2.2:
A sender MUST NOT generate multiple header fields with the same field name in a message unless either the entire field value for that header field is defined as a comma-separated list [i.e., #(values)] or the header field is a well-known exception (as noted below).
So, yeah, I removed the milestone, but I'm not closing the issue either. We're just not rushing to "fix" it before we know the implications. For now, given that there aren't any workarounds to the issues described here, and the fact that so few people have hit them, I'd advise any affected users to comment with their use cases here and :+1: the issue, so we can have more details about the scope and impact of this. In the meantime, you can use xk6 extensions to solve these types of issues for yourself:
As an additional note the HTTP2 RFC specifies that all header field names MUST be lower case. Which IMO makes fixing this even less relevant as the world moves more and more towards http2.
p.s. same is true for HTTP3 at least in the latest draft I could find.
As an additional note the HTTP2 RFC specifies that all header field names MUST be lower case. Which IMO makes fixing this even less relevant as the world moves more and more towards http2.
p.s. same is true for HTTP3 at least in the latest draft I could find.
Since the original issue is that k6 is internally changing the header to some camel-case format, won't you have to fix this to support HTTP2 since it "MUST be lower case"? It seems the better solution would be to leave the headers as is in the script. If an error occurs, the test developers can change the headers to match the requirements for the protocol. As it is now, there is no workaround because k6 is internally changing the headers. Without a workaround, this may eliminate k6 as a viable load testing solution.
Since the original issue is that k6 is internally changing the header to some camel-case format, won't you have to fix this to support HTTP2 since it "MUST be lower case"?
golang's stdlib does this automatically. Otherwise it wouldn't be passing any tests and most servers will have problems with it.
Without a workaround, this may eliminate k6 as a viable load testing solution.
We are aware of this, but the same is true for a lot of the other issues as well. At the moment this seems like a problem for a small percentage of the users, and also a problem that literally will solve itself ... as http1.x becomes less and less common.
I am kind of surpised people still have problems with this ... then again we support ntlm, so I guess I shouldn't be.
And again - if there was some easy fix with no drawbacks - we would likely have implemented it. Unfortunately it seems like it will be quite involved and likely add more problems than not.
As requested by @na-- I come in to report an example of a situation in which I ran into the issue:
b3
header must be lowercase.B3
as we send them)k6/http.request
, the headers would eventually be set using the Go stdlib's http.Header.Set
method, which capitalizes them.Further, I also discovered that as soon as one uses http.Header.Set
or http.Header.Add
methods, Go uses a pretty intrusive normalization strategy. For instance, http.Header.Set("X-B3-ParentSpanId", "foo")
turns the header field name into X-B3-Parentspanid
.
Outcome: I don't necessarily expect the k6/http
to change its behavior for the above reasons. As a user with knowledge of k6's internal, I expect this to become fixed organically if Go ends up aligning the behavior of its HTTP implementation on the HTTP2 specification.
This is the first time in ten years of using the language that I have this "nonsense" feeling about a Go standard API. As a user and a maintainer, I prefer for k6's http.request
function to use direct assignment headers[key] = value
over the Set
and Add
methods.
edited☝🏻to reflect my change of opinion regarding the matter.
We have recently met with the k6 maintainers team to discuss some potential solutions for this.
no decisions have been made, and the following is there as information to feed further research and investigation.
parseRequest
One way to address the issue that's been discussed was to make the http
module's currently private parseRequest
method public, to offer the ability to modules such as k6/experimental/tracing
to produce their request method with a specific behavior: such as one treating headers in a case-insensitive manner.
rawHeaders
Another way we discussed was to add a rawHeaders
attribute to the params
of the http.request
function. This attribute would consist of an Object
(or Map
) that users would be expected to fill with the headers they wish to attach to requests. Those rawHeaders
would be sent as-is, respecting the casing provided by the user. Those rawHeaders
would supersede the headers
field. Passing both attributes would lead to an error.
Another idea we discussed was to establish a distinction between headers
passed as an Object
, which would keep behaving the way they do now, and passing it as a Map
, in which case it would be treated as raw headers, similar to what was discussed .
One conclusion of the discussion was that ideally, we would favor a solution that involves no, or very limited, changes to the current UX.
An important bit of information inherited from that discussion was that Goja Object
protect the ordering of the keys, as they were inserted.
It is not possible to test a non-conformant HTTP server that is handling headers in a case-sensitive manner. The following example script:
Results in the header being sent as
Lower-Case-Header
, meaning that if the server checks the headers in a case-sensitive manner then the header becomes impossible to set in k6.