x/net/http2: requests with absolute URIs in URL.Opaque produce incorrect :path header #16847

saljam · 2016-08-23T11:02:50Z

go version go1.7 darwin/amd64

If one sets URL.Opaque in a request to an absolute URI or a scheme-relative one (e.g. "//example.com/foo"), that request would work over HTTP/1.1, but fail over HTTP/2 on some servers (e.g. NGINX) with a PROTOCOL_ERROR.

func main() {
    req, _ := http.NewRequest("GET", "https://nginx.0f.io/hello", nil)
    req.URL.Opaque = "//nginx.0f.io/hello"
    resp, err := http.DefaultClient.Do(req)
    if err != nil {
        log.Fatal(err)
    }
    log.Println(resp.Proto)
}

$ GODEBUG=http2client=0 go run play.go 
2016/08/23 11:53:48 HTTP/1.1
$ GODEBUG=http2client=1 go run play.go 
2016/08/23 11:53:50 Get https://nginx.0f.io/: stream error: stream ID 1; PROTOCOL_ERROR
exit status 1

That's because x/net/http2 uses URL.Opaque (if present) for the :path header. And while HTTP/1.1 allows a Request-URI to be an absolute URI, HTTP/2 expects the :path header to be just the path part. URL.Opaque now has two slightly different meanings in the context of http.

I'm happy to write a CL to for http2 to ignore URL.Opaque for :path, or maybe parse it and use its path part. That would make existing packages that use Opaque (e.g. googleapi) not break on the upgrade to http2.

But perhaps just documenting this Opaque caveat is sufficient.

http2debug=1 output:

$ GODEBUG=http2debug=1 go run play.go 
2016/08/23 11:54:41 http2: Transport failed to get client conn for nginx.0f.io:443: http2: no cached connection was available
2016/08/23 11:54:41 http2: Transport creating client conn 0xc420136000 to [2400:cb00:2048:1::681c:50d]:443
2016/08/23 11:54:41 http2: Transport encoding header ":authority" = "nginx.0f.io"
2016/08/23 11:54:41 http2: Transport encoding header ":method" = "GET"
2016/08/23 11:54:41 http2: Transport encoding header ":path" = "https://nginx.0f.io/"
2016/08/23 11:54:41 http2: Transport encoding header ":scheme" = "https"
2016/08/23 11:54:41 http2: Transport encoding header "accept-encoding" = "gzip"
2016/08/23 11:54:41 http2: Transport received SETTINGS len=18, settings: MAX_CONCURRENT_STREAMS=128, INITIAL_WINDOW_SIZE=65536, MAX_FRAME_SIZE=16777215
2016/08/23 11:54:41 http2: Transport encoding header "user-agent" = "Go-http-client/2.0"
2016/08/23 11:54:41 http2: Transport received WINDOW_UPDATE len=4 (conn) incr=2147418112
2016/08/23 11:54:41 http2: Transport received SETTINGS flags=ACK len=0
2016/08/23 11:54:41 http2: Transport received RST_STREAM stream=1 len=4 ErrCode=PROTOCOL_ERROR
2016/08/23 11:54:41 RoundTrip failure: stream error: stream ID 1; PROTOCOL_ERROR
2016/08/23 11:54:41 Get https://nginx.0f.io/: stream error: stream ID 1; PROTOCOL_ERROR
exit status 1

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

buro9 · 2016-08-23T11:24:42Z

Worth noting that the Google-API generated clients can not be currently generated in Go1.7 due to this bug.

The generated code will always call SetOpaque (via Expand or directly) https://github.com/google/google-api-go-client/blob/master/google-api-go-generator/gen.go#L1909-L1918 and this results in the "stream ID 1; PROTOCOL_ERROR" at runtime.

However this is not necessarily a google-api generator bug, as it affects all use of Opaque with http/2 when an absolute URI is used.

quentinmit · 2016-08-23T17:17:42Z

/cc @bradfitz

quentinmit · 2016-08-23T17:18:32Z

This seems like it's potentially a regression in 1.7. @bradfitz to weigh in on whether this belongs in a 1.7.1

bradfitz · 2016-08-23T17:58:15Z

Go 1.6 has the same behavior.

@saljam, @buro9, are either of you implying this is a regression from Go 1.6?

@buro9, I'm not sure why you're talking about problems "generating" the code ("can not be currently generated in Go1.7 due to this bug"). This is purely a runtime issue.

Why didn't we hear about it during Go 1.6 if it affects google-api-go-client?

/cc @broady @okdave @mcgreevy

saljam · 2016-08-23T18:06:36Z

@bradfitz this has been the behaviour in http2 for a while, and go1.6.2 exhibits this as well. I wouldn't call it a regression.

So far I've only managed to reproduce it with nginx as the server. A Go http2 server is more lenient in how it parses :path and the request wouldn't fail. Maybe this is why it slipped past.

bradfitz · 2016-08-23T18:09:14Z

Well, we're definitely violating the protocol:

The :path pseudo-header field includes the path and query parts of the target URI (the path-absolute production and optionally a '?' character followed by the query production (see Sections 3.3 and 3.4 of [RFC3986]). A request in asterisk form includes the value '*' for the :path pseudo-header field.

Where RFC 3986 says:

      path          = path-abempty    ; begins with "/" or is empty
                    / path-absolute   ; begins with "/" but not "//"

broady · 2016-08-23T18:14:22Z

Definitely interesting that this hasn't come up with google-api-go-client. I don't know how that happened. Maybe the Google HTTP servers don't have a problem with this.

I can't recommend this for 1.7.1, but don't feel strongly about it.

If I'm reading correctly, the workaround is to not use Opaque. Instead, use a combination of Host/RawPath/RawQuery.

bradfitz · 2016-08-23T18:20:03Z

When I use google-api-go-client's SetOpaque function,

func SetOpaque(u *url.URL) {
        u.Opaque = "//" + u.Host + u.Path
}

on,

        req, _ := http.NewRequest("GET", "https://www.google.com/humans.txt", nil)
        SetOpaque(req.URL)

What happens is that http2 package uses req.URL.RequestURI() to generate the :path and that generates the string "https://www.google.com/humans.txt".

Google GFE surprisingly accepts that:

2016/08/23 18:12:12 http2: Transport encoding header ":authority" = "www.google.com"
2016/08/23 18:12:12 http2: Transport encoding header ":method" = "GET"
2016/08/23 18:12:12 http2: Transport encoding header ":path" = "https://www.google.com/humans.txt"
2016/08/23 18:12:12 http2: Transport encoding header ":scheme" = "https"

... no clue why. Maybe for compatibility for broken people like us. I'll ask.

So @buro9, what issue are you talking about? google-api-go-client seems to work. Or are you using it against non-Google hosts for things like go-endpoints?

bradfitz · 2016-08-23T18:38:25Z

(Note to self: I filed internal bug http://b/31037249 to ask why the Google GFE accepts this.)

bradfitz · 2016-08-23T18:47:08Z

I'm included to do nothing here. Or at least wait until Go 1.8 for any fix.

There's no good place to document this, so I could just fix it in the common case: if we would send a :path starting with https://HOST/, replace it with /. But not if the HOST != Request.URI.Host, to be conservative probably. In that case I'd just return an error early rather than violate the protocol.

@broady opened googleapis/google-api-go-client#161 to update google-api-go-client using the Go 1.5 API added to replace Opaque.

buro9 · 2016-08-23T18:47:44Z

So @buro9, what issue are you talking about? google-api-go-client seems to work. Or are you using it against non-Google hosts for things like go-endpoints?

Yes :)

The generator that is part of the google API go client project is used by other companies to generate their own Go clients.

At CloudFlare we use it to generate a Go client for a 3rd party system that is documented by JSON Schema, and so we are then using Go http/2 to call a Nginx http/2 server that fronts a Glassfish of the 3rd party API.

This specific project has been stuck on Go1.6.1 for a while not knowing where the root cause of the issue was. Salman and I finally looked into it and got to the bottom of it... Opaque handling in http/2.

bradfitz · 2016-08-23T18:50:04Z

Okay, good to know what's happening. You two can follow along at googleapis/google-api-go-client#161 for a short-term fix for go-endpoints.

I'll do a low-priority fix for Go 1.8.

gopherbot · 2016-08-23T21:00:44Z

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

Historically the google-api-go-client has had trouble sending certain characters in paths without the Go standard library (net/url and net/http packages) double escaping or escaping in an unexpected manner (for example, space encoding in issue #64). As a workaround, we started escaping the URL manually and using url.Opaque (for example, 02cfcab and 5c258d4). This mostly works but has problems with HTTP/2 (golang/go#16847). In Go 1.5, the URL struct introduced the RawPath field which was more suitable for this task: if RawPath is provided and is a valid escaping of Path, then the url package will use it as the value of EscapedPath (and EscapedPath is then subsequently used when constructing HTTP requests etc.). This commit changes uritemplates.Expand to return both the unescaped and escaped forms of the the template expansion. This allows us to fill in both url.Path and url.RawPath in a way that satisfies the criteria for url.EscapedPath to function correctly. Issue #161. Change-Id: I51e54e18f198b6465a6d032b1072282bf3d2f9ce Reviewed-on: https://code-review.googlesource.com/7110 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

gopherbot · 2016-09-13T19:00:46Z

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

Updates x/net/http2 (and x/net/lex/httplex) to git rev 749a502 for: http2: don't sniff first Request.Body byte in Transport until we have a conn https://golang.org/cl/29074 Fixes #17071 http2: add Transport support for unicode domain names https://golang.org/cl/29071 Updates #13835 http2: don't send bogus :path pseudo headers if Request.URL.Opaque is set https://golang.org/cl/27632 + http2: fix bug where '*' as a valid :path value in Transport https://golang.org/cl/29070 Updates #16847 http2: fix all vet warnings https://golang.org/cl/28344 Updates #16228 Updates #11041 Also uses the new -underscore flag to x/tools/cmd/bundle from https://golang.org/cl/29086 Change-Id: Ica0f6bf6e33266237e37527a166a783d78c059c4 Reviewed-on: https://go-review.googlesource.com/29110 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org>

Hit golang/go#16847 while cloning the kernel sources. whee. Signed-off-by: Eric Myhre <hash@exultant.us>

It was discovered the bug golang/go#16847 prevents usage of AWS services using HTTP 2, such as AWS X-Ray with the AWS SDK for Go because Go 1.6.2 - 1.7.4 does not correctly build HTTP2 request when the URL uses Opaque to set the escaped version of the URL path. This has been fixed int he Go 1.8 branch, but this change will ensure users using the current version of Go will be able to connect to AWS services using HTTP2. With this change the SDK will no longer with with the unsupported Go version 1.4.

… set Fixes golang/go#16847 Change-Id: I1e6ae1e0746051fd4cf7afc9beae7853293d5b68 Reviewed-on: https://go-review.googlesource.com/27632 Reviewed-by: Chris Broadfoot <cbro@golang.org>

quentinmit added this to the Go1.7.1 milestone Aug 23, 2016

bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 23, 2016

bradfitz modified the milestones: Unreleased, Go1.7.1 Aug 23, 2016

bradfitz added NeedsFix The path to resolution is known, but the work has not been done. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Aug 23, 2016

gopherbot closed this as completed in golang/net@3a1f9ef Aug 25, 2016

warpfork added a commit to polydawn/redesigned-potato that referenced this issue Dec 8, 2016

In which one tries to kernel headers and glibc. Not quite.

0aad4c3

Hit golang/go#16847 while cloning the kernel sources. whee. Signed-off-by: Eric Myhre <hash@exultant.us>

jasdel mentioned this issue Dec 12, 2016

private/protocol/rest: Use RawPath instead of Opaque aws/aws-sdk-go#993

Merged

svanharmelen mentioned this issue Jan 16, 2017

Problem with Repositories.Archive() [STREAM error] xanzy/go-gitlab#114

Closed

bradfitz mentioned this issue Feb 15, 2017

x/net/http2: http get with a path starting by // started failing in 1.8 #19103

Closed

zaksoup mentioned this issue May 22, 2017

Disable HTTP/2 on GoogleClient cloudfoundry/bosh-google-cpi-release#183

Closed

golang locked and limited conversation to collaborators Sep 13, 2017

gopherbot added the FrozenDueToAge label Sep 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/net/http2: requests with absolute URIs in URL.Opaque produce incorrect :path header #16847

x/net/http2: requests with absolute URIs in URL.Opaque produce incorrect :path header #16847

saljam commented Aug 23, 2016

buro9 commented Aug 23, 2016

quentinmit commented Aug 23, 2016

quentinmit commented Aug 23, 2016

bradfitz commented Aug 23, 2016

saljam commented Aug 23, 2016

bradfitz commented Aug 23, 2016

broady commented Aug 23, 2016 •

edited

bradfitz commented Aug 23, 2016

bradfitz commented Aug 23, 2016

bradfitz commented Aug 23, 2016

buro9 commented Aug 23, 2016

bradfitz commented Aug 23, 2016

gopherbot commented Aug 23, 2016

gopherbot commented Sep 13, 2016

x/net/http2: requests with absolute URIs in URL.Opaque produce incorrect :path header #16847

x/net/http2: requests with absolute URIs in URL.Opaque produce incorrect :path header #16847

Comments

saljam commented Aug 23, 2016

buro9 commented Aug 23, 2016

quentinmit commented Aug 23, 2016

quentinmit commented Aug 23, 2016

bradfitz commented Aug 23, 2016

saljam commented Aug 23, 2016

bradfitz commented Aug 23, 2016

broady commented Aug 23, 2016 • edited

bradfitz commented Aug 23, 2016

bradfitz commented Aug 23, 2016

bradfitz commented Aug 23, 2016

buro9 commented Aug 23, 2016

bradfitz commented Aug 23, 2016

gopherbot commented Aug 23, 2016

gopherbot commented Sep 13, 2016

broady commented Aug 23, 2016 •

edited