Skip to content

net/http/httputil: ReverseProxy does not set flushinterval to -1 for Content-Type: text/event-stream in all cases #47359

@cmfcmf

Description

@cmfcmf

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

1.13. The issue/oversight seems to exists at least since 1.12, since the corresponding functionality was introduced here: 5440bfc

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

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/cmfcmf/.cache/go-build"
GOENV="/home/cmfcmf/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/cmfcmf/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.13"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.13/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/cmfcmf/dev/go/src/go.mod"
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-build362045526=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Originally, I was using the Caddy web server and noticed that websocket streams were not correctly reverse-proxied when their Content-Type header included a charset parameter (text/event-stream;charset=utf-8). See caddyserver/caddy#3765 for the issue with Caddy.
I am raising the same issue here, since Caddy's code is based on the code found in go, which appears to have the same problem/oversight.

What did you expect to see?

I expected my reverse-proxied websocket connection to work, regardless of whether or not the optional charset parameter is specified.

What did you see instead?

The connection did not work, since the request/response was not flushed.


This only happens when the websocket server uses the RFC 1521 optional "charset" parameter as part of the Content-Type header. This parameter is also documented here: https://www.w3.org/TR/eventsource/#text-event-stream. Webservers that use this parameter will send a content-type header like text/event-stream;charset=utf-8.
The code causing this problem is located here:

func (p *ReverseProxy) flushInterval(res *http.Response) time.Duration {
resCT := res.Header.Get("Content-Type")
// For Server-Sent Events responses, flush immediately.
// The MIME type is defined in https://www.w3.org/TR/eventsource/#text-event-stream
if resCT == "text/event-stream" {
return -1 // negative means immediately
}
// We might have the case of streaming for which Content-Length might be unset.
if res.ContentLength == -1 {
return -1
}
return p.FlushInterval
}

You can see how it returns -1 for responses with a text/event-stream content type. It should be extended to ignore the charset part of the Content-Type header, similar to what I proposed in my pull request for Caddy: https://github.com/caddyserver/caddy/pull/3758/files, so that it also returns -1 for responses with a text/event-stream;charset=utf-8content-type header.

I have a patch ready to fix that (very similar to the patch I proposed for Caddy), which I can submit on Gerrit/GitHub. However, as per the contribution guidelines, I wanted to open an issue first to discuss potential implications.

Metadata

Metadata

Assignees

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

Relationships

None yet

Development

No branches or pull requests

Issue actions