Navigation Menu

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: TimeoutHandler is incompatible with a ReverseProxy that has a negative FlushInterval #34198

Closed
hochhaus opened this issue Sep 9, 2019 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@hochhaus
Copy link

hochhaus commented Sep 9, 2019

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

1.13.0 fails
1.12.9 works correctly

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="/home/ahochhaus/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="[elided]"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go-1.13"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.13/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build921412516=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run the following example code under both go 1.13 and 1.12.9.

package main

import (
  "log"
  "net/http"
  "net/http/httputil"
  "net/url"
  "time"
)

func main() {
  // setup base http server                                                     
  http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
    http.Redirect(w, r, "/redirectURL", http.StatusFound)
  })
  go func() { log.Fatal(http.ListenAndServe("localhost:1024", nil)) }()

  // setup reverse proxy                                                        
  url, err := url.Parse("http://localhost:1024/")
  if err != nil {
    log.Fatal(err)
  }
  rp := httputil.NewSingleHostReverseProxy(url)
  rp.FlushInterval = -1
  rpServ := &http.Server{
    Addr:    "localhost:1025",
    Handler: http.TimeoutHandler(rp, time.Minute, "msg"),
  }
  go func() { log.Fatal(rpServ.ListenAndServe()) }()

  // send request                                                               
  c := &http.Client{
    CheckRedirect: func(req *http.Request, via []*http.Request) error {
      return http.ErrUseLastResponse
    },
  }
  resp, err := c.Get("http://localhost:1025/")
  if err != nil {
    log.Fatal(err)
  }
  if resp.StatusCode != http.StatusFound {
    log.Fatalf("expected %d. found: %d", http.StatusFound,
      resp.StatusCode)
  }
  log.Printf("success: %d", resp.StatusCode)
}

What did you expect to see?

2019/09/09 17:10:11 success: 302

What did you see instead?

2019/09/09 17:10:11 expected 302. found: 200
@hochhaus hochhaus changed the title net/http: http.TimeoutHandler is incompatible with a ReverseProxy that has a negative FlushInterval net/http: TimeoutHandler is incompatible with a ReverseProxy that has a negative FlushInterval Sep 9, 2019
@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 10, 2019
@agnivade agnivade added this to the Go1.14 milestone Sep 10, 2019
@agnivade
Copy link
Contributor

@bradfitz

@pam4
Copy link

pam4 commented Sep 26, 2019

@hochhaus, the incorrect return code problem should be fixed in 4faf8a8.
But please note that anything on top of a TimeoutHandler would still be unable to flush (including your ReverseProxy, regardless of your FlushInterval setting).

Leaving implementation difficulties aside, a TimeoutHandler simply cannot flush while still being able to return 503 + msg on timeout as advertised. It always buffers the whole response.

TimeoutHandler is useful if you want to limit the running time of your handler (as long as your handler aborts on write errors), but note that the actual sending of the data can still take any amount of time (unless the global Server.WriteTimeout is set).

Unfortunately there is still no way to set different write timeouts for different endpoints, nor to (re)set the timeout from a running handler, see #16100.

Also note that your repro program is racy; you should at least introduce a delay before the Get. And you shouldn't be able to observe the return code problem with Go 1.12.9 because timeoutWriter.Flush was not present yet.

@hochhaus
Copy link
Author

Thanks @pam4. I'll update my code accordingly.

@golang golang locked and limited conversation to collaborators Sep 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants