Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--http-debug incorrectly reports http protocol #986

Open
mstoykov opened this issue Apr 5, 2019 · 1 comment
Open

--http-debug incorrectly reports http protocol #986

mstoykov opened this issue Apr 5, 2019 · 1 comment
Labels

Comments

@mstoykov
Copy link
Contributor

mstoykov commented Apr 5, 2019

--http-debug uses httputil.DumpRequestOut which as mention in the httputil.DumpRequest dumps http2 requests as HTTP/1.1 this is ... at best confusing.

We should probably dump the request after the real connection, although my quick test showed this did not help :)

na-- added a commit that referenced this issue Aug 6, 2019
This makes the code slighly saner and fixes the bug where the first NTLM and digest authentication requests weren't displayed. But by no means does this fix all of the bugs in the http-debug code... For those, see:
 - #986
 - #1042
 - #774

 A discussion how some or all of them can be fixed can be found here: #1102 (comment)
na-- added a commit that referenced this issue Aug 6, 2019
…#1102)

Previously, there was an uncaught error (sometimes leading to panics) in the automatic response body decompression. This patch handles these decompression errors and even assigns a custom error code for them. As a bonus, k6 now also properly handles errors due to improper redirects, or too many redirects, and tags the resulting metric samples accordingly.

By necessity, I had to also move the digest authentication and http-debug handling to their own http.RoundTripper layers. This by no means solves any of the following issues:
 - #800
 - #986
 - #1042
 - #774

...but it hopefully slightly improves the situation. For the digest authentication, a proper authentication cache, and very likely a different library, still have to be used... And regarding `http-debug`, a discussion about how some or all the remaining issues can be fixed can be found here: #1102 (comment)
@mstoykov
Copy link
Contributor Author

We should probably dump the request after the real connection, although my quick test showed this did not help :)

Tried again, and you also need to change to DumpRequest from DumpRequestOut as the later does in practice "sent" the request locally to see what the stdlib will do so 👍 on this.

Unfortunately this also masks anything we set from the response.

diff --git a/lib/netext/httpext/httpdebug_transport.go b/lib/netext/httpext/httpdebug_transport.go
index f242dfcd..c82632e3 100644
--- a/lib/netext/httpext/httpdebug_transport.go
+++ b/lib/netext/httpext/httpdebug_transport.go
@@ -23,14 +23,18 @@ type httpDebugTransport struct {
 //   - https://github.com/k6io/k6/issues/774
 func (t httpDebugTransport) RoundTrip(req *http.Request) (*http.Response, error) {
        id, _ := uuid.NewV4()
-       t.debugRequest(req, id.String())
        resp, err := t.originalTransport.RoundTrip(req)
+       if resp != nil {
+               req.ProtoMajor = resp.ProtoMajor
+               req.ProtoMinor = resp.ProtoMinor
+       }
+       t.debugRequest(req, id.String())
        t.debugResponse(resp, id.String())
        return resp, err
 }

 func (t httpDebugTransport) debugRequest(req *http.Request, requestID string) {
-       dump, err := httputil.DumpRequestOut(req, t.httpDebugOption == "full")
+       dump, err := httputil.DumpRequest(req, t.httpDebugOption == "full")
        if err != nil {
                t.logger.Error(err)
        }

But this now will not showcase the remaining parts the transport will add if not set - so we likely need to set them ...

This might also help #1042, although this might be a bit of breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants