reverse_proxy: fix bi-h2stream breaking gzip encode handle(#3606). #3620
Conversation
| // in that case, if body size larger than enc.MinLength, | ||
| // upper level encode handle might have Content-Encoding header to write. | ||
| // (see https://github.com/caddyserver/caddy/issues/3606 for use case) | ||
| if req.ProtoMajor == 2 && res.ContentLength == -1 && req.Header.Get("Accept-Encoding") == "identity" { |
There was a problem hiding this comment.
If Accept-Encoding is empty (not specified by the client), would that also be permissible for flushing?
There was a problem hiding this comment.
According to rfc2616-sec14, it's permissible.
Should we consider dealing with the following form? It means accepting all but "identity".
Accept-Encoding: identity;q=0, *;q=1
| return -1 | ||
| } | ||
|
|
||
| // TODO: more specific cases? e.g. res.ContentLength == -1? (this TODO is from the std lib) | ||
| return time.Duration(h.FlushInterval) | ||
| } | ||
|
|
||
| // biStream returns whether we should work in bi-directional stream mode. | ||
| func (h Handler) biStream(req *http.Request, res *http.Response) bool { |
There was a problem hiding this comment.
I think the function name should be isBidirectionalStream. Clarifies that the result is a boolean, and "bistream" doesn't have meaning on its own.
|
@masknu This looks great! I took the liberty of applying the last few minor cleanups. If it looks good to you I'm ready to merge this. |
|
That looks great to me. |
|
Thank you so much for your contribution and for the discussion! Please contribute again. 🙂 |
Sorry for breaking the encode handle. This PR fixes #3606 . Add an integration test.