devsisters / goquic

QUIC support for Go
http://devsisters.github.io/goquic/
BSD 3-Clause "New" or "Revised" License
944 stars 100 forks source link

goquic/example/client.go does not seem to work with any real-world QUIC apps #21

Closed gripedthumbtacks closed 8 years ago

gripedthumbtacks commented 8 years ago

I tested this:

github.com/devsisters/goquic/example/client.go

But it cannot connect to any real-world services, like https://www.google.com:443/. I modified it as such to no avail. Here is the error output:

$ go run client.go 2>&1 | egrep -i 'warn|err|fail'
  RREJ: SERVER_CONFIG_INCHOATE_HELLO_FAILURE
[0307/034553:VERBOSE1:quic_connection.cc(2030)] Client: Force closing 9814791807430471583 with error QUIC_PEER_GOING_AWAY (16) Client disconnectiong
[0307/034553:VERBOSE1:quic_packet_creator.cc(740)] Adding frame: type { CONNECTION_CLOSE_FRAME } error_code { 16 } error_details { Client disconnectiong }
<html><title>Error 400 (Bad Request)!!1</title></html>

Has this been tested on any real-world services other than the demo server? Or is QUIC too much in flux still? If the latter, totally understandable. If not, wanted to point it out so others have a documented place to seek resolution. I also noticed this comment in the base client.go foundation at github.com/devsisters/goquic/client.go:

if q.keepConnection {
                resp.Body = ioutil.NopCloser(st)
        } else {
                // XXX(hodduc): "conn" should be closed after the user reads all response body, so
                // it's hard to determine when to close "conn". So we read all response body prematurely.
                // If response is very big, this could be problematic. (Consider using runtime.finalizer())
                body, err := ioutil.ReadAll(st)
                if err != nil {
                        return nil, err
                }
                resp.Body = ioutil.NopCloser(bytes.NewBuffer(body))

                conn.Close()
        }

Can you just use defer on the close? If not, something like this below instead via io.Copy?

body, err := ioutil.ReadAll(resp.Body)
io.Copy(ioutil.Discard, resp.Body)
resp.Body.Close()
hodduc commented 8 years ago

:+1: It worked at that time, but seems to be broken sometime.... The problem is invalid request header. I found that :host header is not valid anymore, and :authority header is necessary according to HTTP2 standard. After fac12bb, https://www.google.com:443/ works well with example client.

serialx commented 8 years ago

Can you try it out and close the issue when it's verified that it's fixed?