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

net/http/httptrace: GotFirstResponseByte can fire twice for HTTP/2 after 100-continue response #15777

Closed
bradfitz opened this issue May 20, 2016 · 2 comments

Comments

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented May 20, 2016

I just noticed that the httptrace.ClientTrace.GotFirstResponseByte hook can fire twice for HTTP/2 connections when the client sent an "Expect: 100-continue" request.

The standard library's net/http tests should check for exactly one occurrence of the substrings in TestTransportEventTrace*, and the http2 code should track whether it already called that hook.

I disappear for a month starting in a few hours. Maybe somebody can fix for 1.7. If not, low priority. Maybe it could even be documented instead for now.

/cc @tombergan (who doesn't care about HTTP/2 I don't think) and @adg

@gopherbot
Copy link

@gopherbot gopherbot commented May 28, 2016

CL https://golang.org/cl/23527 mentions this issue.

@gopherbot
Copy link

@gopherbot gopherbot commented May 28, 2016

CL https://golang.org/cl/23526 mentions this issue.

gopherbot pushed a commit that referenced this issue Jun 1, 2016
Updates x/net/http2 to git rev 6bdd4be4 for CL 23526:

  http2: GotFirstResponseByte hook should only fire once

Also updated the trace hooks test to verify that all trace hooks are called
exactly once except ConnectStart/End, which may be called multiple times (due
to happy-eyeballs).

Fixes #15777

Change-Id: Iea5c64eb322b58be27f9ff863b3a6f90e996fa9b
Reviewed-on: https://go-review.googlesource.com/23527
Reviewed-by: Andrew Gerrand <adg@golang.org>
@golang golang locked and limited conversation to collaborators May 31, 2017
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
Fixes golang/go#15777

Change-Id: I6fd8a8fdea051f3c02272cc15af7f0bcdfb5d745
Reviewed-on: https://go-review.googlesource.com/23526
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.