-
Notifications
You must be signed in to change notification settings - Fork 18.3k
Description
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 envGO111MODULE=""
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:
go/src/net/http/httputil/reverseproxy.go
Lines 401 to 416 in 052da57
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-8
content-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.