net/http: document explicitly that http2 doesn't support Hijacker

[clementino]

When I upgraded to go1.6 (Linux amd64) our applications, which rely heavily on Server-sent events, stopped working because *http2responseWriter does not implement http.Hijacker (#14797) (tried Firefox only).

I'm not using any SSE package, see code below (slightly edited), and it worked fine for 4 years until go1.6.

As I said in the referenced issue, I think there is a problem with the documentation because I relied on a feature that stopped working without warning.
I propose to document that *http2responseWriter is not an Hijacker and any other possible case, so that people can evaluate if and when to rely on the hijacking feature and how to set up the server so that it may fail only for external causes.
It may seems that there are no sensible uses for hijacking besides websockets, but you never know, in fact I was using it for SSE.

EDIT: I realize I can't expect my code to work across http versions if I hardcode the version myself, but even if the version was correct it doesn't reach past the type assertion. How should I go about rewriting this without hijacking?

var sseHeader = "HTTP/1.1 200 OK\nContent-Type: text/event-stream\n\n"

func handleSse(w http.ResponseWriter, r *http.Request) {
    conn, bufrw, err := w.(http.Hijacker).Hijack()
    if err != nil {
        http.Error(w, "hijack failed", http.StatusInternalServerError)
        return
    }
    if _, err := bufrw.WriteString(sseHeader); err != nil {
        conn.Close()
        return
    }
    if err := bufrw.Flush(); err != nil {
        conn.Close()
        return
    }
    newSse(func(msg []byte) (ok bool) {
        for _, line := range bytes.Split(msg, []byte("\n")) {
            if _, err := bufrw.WriteString("data: "); err != nil {
                conn.Close()
                return
            }
            if _, err := bufrw.Write(line); err != nil {
                conn.Close()
                return
            }
            if err := bufrw.WriteByte('\n'); err != nil {
                conn.Close()
                return
            }
        }
        if err := bufrw.WriteByte('\n'); err != nil {
            conn.Close()
            return
        }
        if err := bufrw.Flush(); err != nil {
            conn.Close()
            return
        }
        return true
    })
}

[bradfitz]

The docs on https://golang.org/pkg/net/http/#Hijacker do already say "The Hijacker interface is implemented by ResponseWriters that allow an HTTP handler to take over the connection", which is trying to say that not all ResponseWriters support that. If they did, it would just be part of the ResponseWriter interface.

To do this without hijack, just write to your ResponseWriter and use https://golang.org/pkg/net/http/#Flusher instead. Both http1 and http2 support Flusher (but you should still test for it and conditionally use it). There's no need to use a buffered writer. The http1 and http2 response writers are already buffered. (and any unbuffered one wouldn't need a flush)

[bradfitz]

I'll repurpose this bug to be about documentation.

[clementino]

Thanks, your solution works fine!
I thought that besides flushing, without hijacking I wouldn't be able to disable chunking in the ResponseWriter and that SSE wouldn't work with chunking enabled.
But I was wrong in both cases: chunking is not recommended for SSE but allowed, and to disable it all I had to do was to change the Transfer-Encoding header.
Sorry.

About the docs I understand. I'm not saying that it is incorrect, I'm saying that it could be more useful.
I don't choose a ResponseWriter over another, I just use the one that is given to me. If a feature depends on some specific ResponseWriter and I depend on such a feature I want to have an idea when it may not be available.

I understand that use cases for Hijacker are limited but I'm glad you decided to document it, thanks.

[gopherbot]

CL https://golang.org/cl/22122 mentions this issue.