Skip to content

Commit 96800ae

Browse files
committed
fix: send the 101 status line before the connection is handed over
WriteHeader only committed when a Content-Type was already set, so the plain upgrade sequence, 101 with no content type followed by Hijack, recorded the status and never forwarded it: Hijack set hijacked and close returned at that guard, leaving the client with the raw protocol bytes and no status line where net/http would have written one. 101 now commits immediately, which it can do because it carries no body to sniff and is already excluded from compression. The regression test omits Content-Type and reads the raw socket, so it fails without the fix. Reported by umputun on #51.
1 parent a287028 commit 96800ae

2 files changed

Lines changed: 43 additions & 2 deletions

File tree

‎gzip.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@ func (w *gzipResponseWriter) WriteHeader(status int) {
6161
w.status = status
6262

6363
// with a content type in hand the decision can be made right away, otherwise it waits for the
64-
// first Write so the body can be sniffed
65-
if ctype := w.Header().Get("Content-Type"); ctype != "" {
64+
// first Write so the body can be sniffed. 101 cannot wait: the upgrade sequence carries no body
65+
// and usually no content type, and the Hijack that follows would leave the status unsent
66+
if ctype := w.Header().Get("Content-Type"); ctype != "" || status == http.StatusSwitchingProtocols {
6667
w.decide(ctype)
6768
w.commit()
6869
}

‎gzip_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"io"
9+
"net"
910
"net/http"
1011
"net/http/httptest"
1112
"strconv"
@@ -791,3 +792,42 @@ func TestGzipWriterCapabilitiesMatchUnderlying(t *testing.T) {
791792
})
792793
}
793794
}
795+
796+
func TestGzipSwitchingProtocolsWithoutContentType(t *testing.T) {
797+
// the plain upgrade sequence: 101 with no content type, then the handler takes the connection.
798+
// the status has to be on the wire before Hijack, or it is never sent at all
799+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
800+
w.Header().Set("Upgrade", "foo")
801+
w.Header().Set("Connection", "Upgrade")
802+
w.WriteHeader(http.StatusSwitchingProtocols)
803+
804+
hj, ok := w.(http.Hijacker)
805+
require.True(t, ok, "upgrade needs a Hijacker")
806+
conn, buf, err := hj.Hijack()
807+
require.NoError(t, err)
808+
defer conn.Close()
809+
810+
_, err = buf.WriteString("raw-protocol-bytes")
811+
require.NoError(t, err)
812+
require.NoError(t, buf.Flush())
813+
})
814+
815+
ts := httptest.NewServer(Gzip()(handler))
816+
defer ts.Close()
817+
818+
conn, err := net.Dial("tcp", strings.TrimPrefix(ts.URL, "http://"))
819+
require.NoError(t, err)
820+
defer conn.Close()
821+
822+
_, err = fmt.Fprint(conn, "GET /upgrade HTTP/1.1\r\nHost: example.com\r\n"+
823+
"Accept-Encoding: gzip\r\nUpgrade: foo\r\nConnection: Upgrade\r\n\r\n")
824+
require.NoError(t, err)
825+
require.NoError(t, conn.SetReadDeadline(time.Now().Add(5*time.Second)))
826+
827+
got, err := io.ReadAll(conn)
828+
require.NoError(t, err)
829+
830+
assert.Contains(t, string(got), "101 Switching Protocols", "the 101 status line must reach the client")
831+
assert.Contains(t, string(got), "raw-protocol-bytes")
832+
assert.NotContains(t, string(got), "Content-Encoding: gzip", "an upgraded connection must not be gzipped")
833+
}

0 commit comments

Comments
 (0)