Skip to content

/ go

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

Jump to bottom

net/http: Flush in TimeoutHandler for Go1.13 is broken and might need a revert [1.13 backport] #34560

Closed
gopherbot opened this issue Sep 26, 2019 · 4 comments
Closed

net/http: Flush in TimeoutHandler for Go1.13 is broken and might need a revert [1.13 backport] #34560

gopherbot opened this issue Sep 26, 2019 · 4 comments
Labels
CherryPickApproved
Milestone
Go1.13.3

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 26, 2019

@bcmills requested issue #34439 to be considered for backport to the next 1.13 minor release.

@gopherbot, please backport to Go 1.13: we should minimize the window during which the erroneous method is present.
@gopherbot gopherbot mentioned this issue Sep 26, 2019
Closed
@gopherbot gopherbot added this to the Go1.13.2 milestone Sep 26, 2019
@bcmills

This comment has been minimized.

Sign in to view
Copy link
Member

@bcmills bcmills commented Sep 26, 2019

In #34439 (comment), @pam4 notes:

timeoutWriter is unexported, so no compile-time incompatibility, and although the method exists, the interface was never actually implemented. Go 1.13 docs made a false/impossible claim and I think we should just take it back. […] any program using the method is already broken

In #34439 (comment), @bradfitz replies:

I'm fine reverting the Flusher stuff, though. That's easier than making it work properly.
@bcmills

This comment has been minimized.

Sign in to view
Copy link
Member

@bcmills bcmills commented Sep 26, 2019

Based on the above, approving for Go 1.13.2.
@gopherbot

This comment has been minimized.

Sign in to view
Copy link
Author

@gopherbot gopherbot commented Sep 26, 2019

Change https://golang.org/cl/197543 mentions this issue: [release-branch.go1.13] net/http, doc/go1.13.html: revert TimeoutHandler.Flush
@gopherbot

This comment has been minimized.

Sign in to view
Copy link
Author

@gopherbot gopherbot commented Sep 27, 2019

Closed by merging de96471 to release-branch.go1.13.
@gopherbot gopherbot closed this Sep 27, 2019
gopherbot pushed a commit that referenced this issue Sep 27, 2019
@odeke-em @bradfitz
[release-branch.go1.13] net/http, doc/go1.13.html: revert TimeoutHand…
de96471 
…ler.Flush

Also added a test to ensure that any interactions
between TimeoutHandler and Flusher result in the
correct status code and body, but also that we don't
get superfluous logs from stray writes as was seen
in the bug report.

Fixes #34560.

Change-Id: I4af62db256742326f9353f98a2fcb5f71d2a5fd9
Reviewed-on: https://go-review.googlesource.com/c/go/+/197659
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 4faf8a8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/197543
@katiehockman katiehockman modified the milestones: Go1.13.2, Go1.13.3 Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CherryPickApproved
Projects
None yet
Milestone
  Go1.13.3
3 participants
@gopherbot @bcmills @katiehockman
You can’t perform that action at this time.