New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/net/http2: *http2responseWriter does not implement http.Hijacker #14797

Closed
wathiede opened this Issue Mar 13, 2016 · 6 comments

Comments

Projects
None yet
6 participants
@wathiede
Copy link
Member

wathiede commented Mar 13, 2016

  1. What version of Go are you using (go version)?
    go version go1.6 freebsd/amd64
  2. What operating system and processor architecture are you using (go env)?
    go version go1.6 freebsd/amd64
  3. What did you do?

I'm not sure if this is a bug report or a feature request, but the response writer used for HTTP2 doesn't support hijacking even though the HTTP/1.1 + TLS does.

Example code:
http://play.golang.org/p/2o6LE5FHj3

Run with:
$ GODEBUG=http2server=0 go run main.go

Response says:

*http.response implements http.Hijack: true

Run with:
$ GODEBUG=http2server=1 go run main.go

Response says:

*http.http2responseWriter implements http.Hijack: false
  1. What did you expect to see?
    I expected http.Handlers that Hijack connections with HTTP/1.x plaintext & TLS to work with HTTP/2 connections.
  2. What did you see instead?
    Code failing because the response writer didn't support hijacking.

CC @bradfitz

@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Mar 14, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Mar 14, 2016

Hijack and HTTP/2 are fundamentally incompatible. HTTP/2 permits multiple requests at a time. What would hijacking a request mean? What protocol do you expect to speak afterwards?

The usual (basically only) reason for wanting Hijack is for websockets, but websockets also don't work over HTTP/2 (due to the IETF working groups forgetting about it until the last second, basically), so browsers fall back to HTTP/1.1 before making a websocket request. You can have a whole site that's only HTTP/2, but once you try to use websockets it'll create a new TCP/TLS connection just to speak a bit of HTTP/1.1, enough do the upgrade to websockets.

If you have some custom client that's not websockets and you're speaking your own protocol, you can either just not use HTTP/2 at all, or you can do bidirectional streaming (streaming the request and streaming the response) from one Go http.Handler to an HTTP/2 client.

I'm closing this because this sounds hypothetical and there's no intention to address this. If you have a valid use case I didn't think of, though, then we can reopen this.

@bradfitz bradfitz closed this Mar 14, 2016

@gaillard

This comment has been minimized.

Copy link

gaillard commented Apr 11, 2016

So in http1.1 I would use hijack as a way to indicate that there was an error writing out data, after already writing some. How would I give a last ditch effort in http2 to tell the client there was a problem since writeheader has already been called?

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Apr 12, 2016

@gaillard, panic in your handler. This works for both http1 and http2, as demonstrated here: https://go-review.googlesource.com/21881 and is kinda documented here: https://golang.org/pkg/net/http/#Handler

This behavior was always true, but the new test locks it in further.

@gaillard

This comment has been minimized.

Copy link

gaillard commented Apr 12, 2016

Ah great thanks!

@clementino

This comment has been minimized.

Copy link

clementino commented Apr 14, 2016

When I upgraded to go1.6 our applications, which rely heavily on Server-sent events, stopped working because of this issue (tried Firefox only).

I was in a hurry and I couldn't find any information but I needed a fix introduced in go1.6, so I just disabled http2. Now I'm looking again and found this.

Does SSE work with http2? If not, why Firefox doesn't fall back to http1.1 for SSE too?

Anyhow I think there is a problem with the documentation.
I assumed a ResponseWriter always implements Hijacker unless otherwise noted but I was wrong, basically the documentation says: a ResponseWriter may or may not implement Hijacker, when and why we wont tell, you discover at runtime.

You say that it just works for every sensible use case so why bother, but you never know. It may save people time to document that http2responseWriter is not an Hijacker (and other possible cases?) so that if I need an Hijacker I can do everything in my power to have one.

https://en.wikipedia.org/wiki/Server-sent_events

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Apr 14, 2016

@clementino, this bug is closed. Please open a new one about SSE. There's no reason SSE shouldn't work with HTTP/2. It's probably a problem with the package you're using. You can tell me more about it in the new bug. You can reference this bug number in your new bug number.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.