net/http: Proto when set is not used properly. #13985

harshavardhana · 2016-01-17T10:12:43Z

For example even after setting request Proto has no affect on how http.Client sends request, it always defaults to. 'HTTP/1.1'

$ cat http-test.go
package main

import (
    "fmt"
    "log"
    "net/http"
    "net/http/httputil"
)

func main() {
    req, err := http.NewRequest("GET", "https://www.google.com", nil)
    if err != nil {
        log.Fatalln(err)
    }
    req.Proto = "HTTP/1.0"
    client := &http.Client{}
    resp, err := client.Do(req)
    if err != nil {
        log.Fatalln(err)
    }
    reqTrace, err := httputil.DumpRequestOut(req, false)
    if err != nil {
        log.Fatalln(err)
    }
    respTrace, err := httputil.DumpResponse(resp, false)
    if err != nil {
        log.Fatalln(err)
    }
    fmt.Println(string(reqTrace))
    fmt.Println(string(respTrace))
}

$ go run http-test.go
GET / HTTP/1.1
Host: www.google.com
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip


HTTP/1.1 200 OK
Transfer-Encoding: chunked
Alt-Svc: quic="www.google.com:443"; ma=600; v="30,29,28,27,26,25",quic=":443"; ma=600; v="30,29,28,27,26,25"
Alternate-Protocol: 443:quic,p=1
Cache-Control: private, max-age=0
Content-Type: text/html; charset=ISO-8859-1
Date: Sun, 17 Jan 2016 10:08:59 GMT
Expires: -1
P3p: CP="This is not a P3P policy! See https://www.google.com/support/accounts/answer/151657?hl=en for more info."
Server: gws
Set-Cookie: NID=75=OeIomPif8UPzAt2QI2qzIOeqiJo4jvQRmsKjBHWDpaXwDGizQB5lAxi0Lb3Dw_5sCeWvZmccGoZER2Pj3zuvY2PI982WCxDQBhU62CW1cZUmMqUNeT5vv5tDA1g_646320jRqqcKxFSuQCg; expires=Mon, 18-Jul-2016 10:08:59 GMT; path=/; domain=.google.com; HttpOnly
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 1; mode=block

Now testing with .Proto as 'HTTP/2.0' defaults to 'HTTP/1.1'

$ cat http-test.go
package main

import (
    "fmt"
    "log"
    "net/http"
    "net/http/httputil"
)

func main() {
    req, err := http.NewRequest("GET", "https://www.google.com", nil)
    if err != nil {
        log.Fatalln(err)
    }
    req.Proto = "HTTP/2.0"
    client := &http.Client{}
    resp, err := client.Do(req)
    if err != nil {
        log.Fatalln(err)
    }
    reqTrace, err := httputil.DumpRequestOut(req, false)
    if err != nil {
        log.Fatalln(err)
    }
    respTrace, err := httputil.DumpResponse(resp, false)
    if err != nil {
        log.Fatalln(err)
    }
    fmt.Println(string(reqTrace))
    fmt.Println(string(respTrace))
}

$ go run http-test.go
GET / HTTP/1.1
Host: www.google.com
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip


HTTP/1.1 200 OK
Transfer-Encoding: chunked
Alt-Svc: quic="www.google.com:443"; ma=600; v="30,29,28,27,26,25",quic=":443"; ma=600; v="30,29,28,27,26,25"
Alternate-Protocol: 443:quic,p=1
Cache-Control: private, max-age=0
Content-Type: text/html; charset=ISO-8859-1
Date: Sun, 17 Jan 2016 10:11:35 GMT
Expires: -1
P3p: CP="This is not a P3P policy! See https://www.google.com/support/accounts/answer/151657?hl=en for more info."
Server: gws
Set-Cookie: NID=75=zVFTFbi8WLNy04TQUSBVFJ6ioFJs-YUrYLI3Js2dZ8cIoiBLLcxAnOOd4REY9Lns3TPZtPjXE5Cdb7C40vqk4HnbDWdSkSp3j0mMc1Z83pFWR22KMvJcofKJJjOVeKv9GZsJ66MRF1vlxGQ; expires=Mon, 18-Jul-2016 10:11:35 GMT; path=/; domain=.google.com; HttpOnly
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 1; mode=block

The text was updated successfully, but these errors were encountered:

harshavardhana · 2016-01-17T10:13:04Z

After looking into the code i see that fix to be

diff --git a/src/net/http/request.go b/src/net/http/request.go
index c2f5f26..446a490 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -413,7 +413,7 @@ func (req *Request) write(w io.Writer, usingProxy bool, extraHeaders Header, wai
                w = bw
        }

-       _, err := fmt.Fprintf(w, "%s %s HTTP/1.1\r\n", valueOrDefault(req.Method, "GET"), ruri)
+       _, err := fmt.Fprintf(w, "%s %s %s\r\n", valueOrDefault(req.Method, "GET"), ruri, valueOrDefault(req.Proto, "HTTP/1.1"))
        if err != nil {
                return err
        }

harshavardhana · 2016-01-17T10:14:39Z

and testing the program again.

$ ../bin/go run http-test.go
GET / HTTP/2.0
Host: www.google.com
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip


HTTP/2.0 200 OK
Connection: close
Alt-Svc: quic="www.google.com:443"; ma=600; v="30,29,28,27,26,25",quic=":443"; ma=600; v="30,29,28,27,26,25"
Alternate-Protocol: 443:quic,p=1
Cache-Control: private, max-age=0
Content-Type: text/html; charset=ISO-8859-1
Date: Sun, 17 Jan 2016 10:14:15 GMT
Expires: -1
P3p: CP="This is not a P3P policy! See https://www.google.com/support/accounts/answer/151657?hl=en for more info."
Server: gws
Set-Cookie: NID=75=WysA4QXJvCMHyi8lQb0olN4eyV1CSxNsO3UjCi1XZ2YRAR2RxUAztVbc67OD_7paNMvjR-skJ8qs-fNPut86CthxCzUOKR0lobBssfxJes6EeS7V2KwCwfbr1fBcwOi-t-xoXpd_Lrw4Ow; expires=Mon, 18-Jul-2016 10:14:15 GMT; path=/; domain=.google.com; HttpOnly
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 1; mode=block

harshavardhana · 2016-01-17T10:16:23Z

Changing to an unsupported server for 2.0

$ cat http-test.go
package main

import (
    "fmt"
    "log"
    "net/http"
    "net/http/httputil"
)

func main() {
    req, err := http.NewRequest("GET", "https://s3.amazonaws.com", nil)
    if err != nil {
        log.Fatalln(err)
    }
    req.Proto = "HTTP/2.0"
    client := &http.Client{}
    resp, err := client.Do(req)
    if err != nil {
        log.Fatalln(err)
    }
    reqTrace, err := httputil.DumpRequestOut(req, false)
    if err != nil {
        log.Fatalln(err)
    }
    respTrace, err := httputil.DumpResponse(resp, false)
    if err != nil {
        log.Fatalln(err)
    }
    fmt.Println(string(reqTrace))
    fmt.Println(string(respTrace))
}

$ ../bin/go run http-test.go
GET / HTTP/2.0
Host: s3.amazonaws.com
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip


HTTP/1.1 505 HTTP Version not supported
Connection: close
Transfer-Encoding: chunked
Content-Type: application/xml
Date: Sun, 17 Jan 16 10:15:35 GMT
X-Amz-Id-2: 6aiu182GZ8ya6mIGI/caABbhy2fYLgi8FQsx7AXQeXzFx2K/ifF55fRz2zgnB9cgA6Y6aQyeEsjU7Hfy1SDpk8y1W/NqkZwN
X-Amz-Request-Id: A7FAF8B8E615F4BE

gopherbot · 2016-01-17T11:00:10Z

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

nussjustin · 2016-01-17T11:00:32Z

The comment on Request.Proto says

// The protocol version for incoming requests.
// Client requests always use HTTP/1.1.

So, yeah there is nothing to fix. The code already works as documented. Also this won't work for HTTP 2 anyway as it's implemented in the golang.org/x/net/http2 repo which is vendored into the net/http package.

The comment on Request.Proto should probably be updated to say that HTTP/2.0 will be used for client requests if available.

bradfitz · 2016-01-17T16:07:23Z

The comment actually had been updated. Read at tip.golang.org/pkg/net/http

bradfitz · 2016-01-17T16:53:17Z

I replied further on https://golang.org/cl/18705

harshavardhana · 2016-01-17T20:11:49Z

The comment actually had been updated. Read at tip.golang.org/pkg/net/http

This is what led to this change. I see the 'keep-alive' change i didn't address.

    // The protocol version for incoming server requests.
    //
    // For client requests these fields are ignored. The HTTP
    // transport code uses either HTTP/1.1 or HTTP/2.0 by default,
    // depending on what the server supports.
    Proto      string // "HTTP/1.0"
    ProtoMajor int    // 1
    ProtoMinor int    // 0

bradfitz · 2016-01-17T20:13:08Z

I don't understand.

harshavardhana · 2016-01-17T20:13:35Z

I don't understand.

`Proto string // "HTTP/1.0"`` is never honored.

bradfitz · 2016-01-17T20:16:11Z

What about it? That's a valid example value you might get for server requests.

harshavardhana · 2016-01-17T20:19:56Z

What about it? That's a valid example value you might get for server requests.

Now i see that i didn't read the comment properly, since client is supposed to automatically handle the incoming requests based on their proto. One of the reasons why one would ignore .Proto field on client side.

That was my confusion, thanks for clarifying.

But is it ever valid to set .Proto after http.NewRequest to some value but the underlying write() is always defaulting to HTTP/1.1 - just wondering if this behavior is correct?

bradfitz · 2016-01-17T20:21:28Z

"For client requests these fields are ignored." I don't know how I can word that more clearly.

harshavardhana · 2016-01-17T20:22:20Z

"For client requests these fields are ignored." I don't know how I can word that more clearly.

Hmm, okay. Understood.

bradfitz · 2016-01-18T06:40:33Z

I actually did think how I can word it more clearly. I sent out https://go-review.googlesource.com/18720

gopherbot · 2016-01-18T07:00:10Z

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

harshavardhana · 2016-01-18T10:18:27Z

I actually did think how I can word it more clearly. I sent out https://go-review.googlesource.com/18720

Thanks.

No need to say "by default" because there is no alternative and no way to override. Always HTTP/2.0 is officially spelled HTTP/2 these days. Fixes #13985 harder Change-Id: Ib1ec03cec171ca865342b8e7452cd4c707d7b770 Reviewed-on: https://go-review.googlesource.com/18720 Reviewed-by: Rob Pike <r@golang.org>

bradfitz closed this as completed Jan 17, 2016

golang locked and limited conversation to collaborators Jan 17, 2017

gopherbot added the FrozenDueToAge label Jan 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net/http: Proto when set is not used properly. #13985

net/http: Proto when set is not used properly. #13985

harshavardhana commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

gopherbot commented Jan 17, 2016

nussjustin commented Jan 17, 2016

bradfitz commented Jan 17, 2016

bradfitz commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

bradfitz commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

bradfitz commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

bradfitz commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

bradfitz commented Jan 18, 2016

gopherbot commented Jan 18, 2016

harshavardhana commented Jan 18, 2016

net/http: Proto when set is not used properly. #13985

net/http: Proto when set is not used properly. #13985

Comments

harshavardhana commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

gopherbot commented Jan 17, 2016

nussjustin commented Jan 17, 2016

bradfitz commented Jan 17, 2016

bradfitz commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

bradfitz commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

bradfitz commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

bradfitz commented Jan 17, 2016

harshavardhana commented Jan 17, 2016

bradfitz commented Jan 18, 2016

gopherbot commented Jan 18, 2016

harshavardhana commented Jan 18, 2016