I'm reluctant to add too much new API to the net/http package at this point. What did you have in mind?

Thanks for your quick answer.

What about something like this?

[...]

type Hijacker interface {
    Hijack() (net.Conn, *bufio.ReadWriter, error)
    Hijacked() bool
}

[...]

func (w *response) Hijacked() bool {
    return w.conn.hijacked()
}

This should be backward compatible change unless i'm missing something.

No, you can't enlarge an interface and still be backwards compatible.

See also: http://blog.merovius.de/2015/07/29/backwards-compatibility-in-go.html

Ups, sorry i missed it.

Ignore what I said, what about another interface (too much?) for the sole purpose of knowing if it's hijacked or not? Please don't take into account the interface name, we can think something that it's more accurate if you feel this can be included.

[...]

type Hijackeder interface {
    Hijacked() bool
}

[...]

func (w *response) Hijacked() bool {
    return w.conn.hijacked()
}

Yeah, any new interface feels like too much. Maybe we can shove it (it == some new func?) away in the httputil package or something. Not sure if that's gross itself, but since it's already a random package, maybe it's okay.

I'm lost, how to get an internal property value in a subpackage which doesn't have parent package visibility? Or i'm completely wrong?

Exposing a boolean (through method/field which breaks compatibility) seems like I'm choosing which one to cut the red or blue wire :)

One option is to declare an unexported helper function in net/http, and then use //go:linkname to make it accessible from net/http/httputil. We use this trick a fair amount in the standard library. That said, it's kind of a hack, so I'd only resort to that if necessary.

@mdempsky, I don't think we'd even need that trick. Both net/http and net/http/httputil could depend on an shared internal package and pass around the func through that.

Actually, something else we could do is just not spew to the server log if lenData == 0, but still return ErrHijacked like we do today:

func (w *response) Write(data []byte) (n int, err error) {
        return w.write(len(data), data, "")
}

func (w *response) WriteString(data string) (n int, err error) {
        return w.write(len(data), nil, data)
}

// either dataB or dataS is non-zero.    
func (w *response) write(lenData int, dataB []byte, dataS string) (n int, err error) {
        if w.conn.hijacked() {
                w.conn.server.logf("http: response.Write on hijacked connection")
                return 0, ErrHijacked
        }

Changing the code to be:

func (w *response) write(lenData int, dataB []byte, dataS string) (n int, err error) {
        if w.conn.hijacked() {
                if lenData > 0 {
                        w.conn.server.logf("http: response.Write on hijacked connection")
                }
                return 0, ErrHijacked
        }

Then you just do a Write(nil) and check the error value, but we still spam the logs in the majority of cases for people making actual coding mistakes, which is why the log was there in the first place.

No new API.

Although it's a little bit obscure, I think that brad's approach it's the best one, at least for the 1.X branch.

I don't mind to create a patch with your solution, do you want me to proceed?

ping @bradfitz

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

Done. I was leaving the easy ones to the end. @quentinmit, want to review?

thanks @bradfitz 👍

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