Skip to content

net/http/httputil: ReverseProxy FlushInterval no longer flushes headers in go1.12 #31125

Closed
Closed
net/http/httputil: ReverseProxy FlushInterval no longer flushes headers in go1.12#31125
Labels
FrozenDueToAgeNeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Description

@liggitt
liggitt
opened on Mar 29, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/liggitt/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/liggitt/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/liggitt/.gvm/gos/go1.12.1"
GOTMPDIR=""
GOTOOLDIR="/Users/liggitt/.gvm/gos/go1.12.1/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/h8/3dzrjpfj76d93vbhc5f2r8tw00kjgb/T/go-build963504992=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

  1. Set up a reverse proxy with a flush interval.
  2. Proxied to a backend that writes and flushes headers, then streams response chunks (which can be arbitrarily delayed from the original header write/flush).

What did you expect to see?

Clients making requests to the proxy should see the headers from the backend no later than the FlushInterval

What did you see instead?

Clients making requests to the proxy receive no data until the first body chunk is written.

https://play.golang.org/p/Y8KQO8-57MR

  • go1.11 works properly
  • go1.12 hangs

The change in 5440bfc#diff-d863507a61be206d112f6e00e6d812a2L343 removed the goroutine that ensured the FlushInterval was honored while blocking in copyResponse, even prior to the first Write() call being encountered. The time.After() ticker it sets up doesn't get set up until the first Write() call is made.

Activity

  1. gopherbot commented on Mar 29, 2019

    @gopherbot
    gopherbot
    on Mar 29, 2019
    Contributor

    Change https://golang.org/cl/170066 mentions this issue: net/http/httputil: make ReverseProxy flush headers on FlushInterval

  2. thaJeztah commented on Mar 29, 2019

    @thaJeztah
    thaJeztah
    on Mar 29, 2019
    Contributor

    Could this be tracked for Go 1.12.2 ? (is a separate tracking issue needed for that?)

  3. ALTree
    changed the title [-]httputil.ReverseProxy FlushInterval no longer flushes headers in go1.12[/-] [+]net/http/httputil: ReverseProxy FlushInterval no longer flushes headers in go1.12[/+] on Mar 29, 2019
  4. ALTree
    added
    NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
    on Mar 29, 2019

  5. ALTree commented on Mar 29, 2019

    @ALTree
    ALTree
    on Mar 29, 2019
    Member

    cc @bradfitz for investigation and decision regarding a possible backport

    is a separate tracking issue needed for that?

    In general, after the issue has been investigated, a team member will decide if it qualifies for a backport. Then a backport issue will be opened.

  6. PaulSD commented on Mar 29, 2019

    @PaulSD
    PaulSD
    on Mar 29, 2019

    Work-around for Go 1.12:

    func HttpHandler(w http.ResponseWriter, req *http.Request) {
  ...
  proxy.ServeHTTP(NewFlushResponseWriter(w), req)
}

type flushResponseWriter struct {
  http.ResponseWriter
  http.Flusher
  http.Hijacker
  http.CloseNotifier
}
func NewFlushResponseWriter(wrap http.ResponseWriter) (http.ResponseWriter) {
  f, _ := wrap.(http.Flusher)
  h, _ := wrap.(http.Hijacker)
  cn, _ := wrap.(http.CloseNotifier)
  if f == nil || h == nil || cn == nil {
    log.Printf("WARNING: http.ResponseWriter doesn't implement all flushResponseWriter interfaces, will not flush response headers")
    return wrap
  }
  return flushResponseWriter{wrap, f, h, cn}
}
func (self flushResponseWriter) WriteHeader(statusCode int) {
  self.ResponseWriter.WriteHeader(statusCode)
  self.Flusher.Flush()
}

  7. bradfitz commented on Mar 29, 2019

    @bradfitz
    bradfitz
    on Mar 29, 2019
    Contributor

    @gopherbot, please backport to Go 1.12.

  8. gopherbot commented on Mar 29, 2019

    @gopherbot
    gopherbot
    on Mar 29, 2019
    Contributor

    Backport issue(s) opened: #31144 (for 1.12).

    Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

  9. gopherbot
    closed this as completedin 2cc3473on Mar 29, 2019
  10. jktomer
    added a commit that references this issue on Oct 7, 2019

    Up minimum go version to 1.12.4

    d34f49b
  11. golang
    locked and limited conversation to collaborators on Mar 28, 2020
  12. gopherbot
    added
    FrozenDueToAge
    on Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeNeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions