search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.06k stars 17.68k forks source link

net/http: 308 response missing Location header #17773

Closed bradfitz closed 8 years ago

bradfitz commented 8 years ago

I just tried to upload a binary to Google Cloud Storage using golang.org/x/build/cmd/upload and got:

2016/11/03 17:20:16 Write error: Post https://www.googleapis.com/upload/storage/v1/b/go-builder-data/o?alt=json&projection=full&uploadType=resumable&upload_id=xxxxx: 308 response missing Location header

I suspect this is due to the recent 307/308 support in 7db996ee77b20b68ee583c65d59be1d81cef9090 (https://go-review.googlesource.com/29852).

/cc @odeke-em @jba

bradfitz commented 8 years ago

Um, Google is actually replying like that:

2016/11/03 17:26:58 http2: Framer 0xc4201dd380: read HEADERS flags=END_STREAM|END_HEADERS stream=5 len=65
2016/11/03 17:26:58 http2: decoded hpack field header field ":status" = "308"  
2016/11/03 17:26:58 http2: decoded hpack field header field "x-guploader-uploadid" = "AEnB2Urzk0RnIpfPa3ajMQ9S6djxJLJ7W2-xxxx-xxxxx"
2016/11/03 17:26:58 http2: decoded hpack field header field "range" = "bytes=0-8388607"
2016/11/03 17:26:58 http2: decoded hpack field header field "x-range-md5" = "e48f3fcb9ae099cfec5f7fd65d9f8c94"
2016/11/03 17:26:58 http2: decoded hpack field header field "content-length" = "0"
2016/11/03 17:26:58 http2: decoded hpack field header field "date" = "Thu, 03 Nov 2016 17:26:58 GMT"
2016/11/03 17:26:58 http2: decoded hpack field header field "server" = "UploadServer"
2016/11/03 17:26:58 http2: decoded hpack field header field "content-type" = "text/html; charset=UTF-8"
2016/11/03 17:26:58 http2: decoded hpack field header field "alt-svc" = "quic=\":443\"; ma=2592000; v=\"36,35,34\""
2016/11/03 17:26:58 http2: Transport received HEADERS flags=END_STREAM|END_HEADERS stream=5 len=65
2016/11/03 17:26:58 http2: Framer 0xc4201dd380: read PING len=8 ping="\x00\x00\x00\x00\x00\x00\x00\x04"
2016/11/03 17:26:58 http2: Transport received PING len=8 ping="\x00\x00\x00\x00\x00\x00\x00\x04"
2016/11/03 17:26:58 http2: Framer 0xc4201dd380: wrote PING flags=ACK len=8 ping="\x00\x00\x00\x00\x00\x00\x00\x04"  
2016/11/03 17:26:58 Write error: Post https://www.googleapis.com/upload/storage/v1/b/go-builder-data/o?alt=json&projection=full&uploadType=resumable&upload_id=xxx: 308 response missing Location header

And this was the request:

2016/11/03 17:26:58 http2: Transport encoding header ":authority" = "www.googleapis.com"
2016/11/03 17:26:58 http2: Transport encoding header ":method" = "POST"
2016/11/03 17:26:58 http2: Transport encoding header ":path" = "/upload/storage/v1/b/go-builder-data/o?alt=json&projection=full&uploadType=resumable&upload_id=xxxxx-xxxxxx"
2016/11/03 17:26:58 http2: Transport encoding header ":scheme" = "https"
2016/11/03 17:26:58 http2: Transport encoding header "authorization" = "Bearer ya29.xxxxxxxxxx"
2016/11/03 17:26:58 http2: Transport encoding header "content-range" = "bytes 0-8388607/*"
2016/11/03 17:26:58 http2: Transport encoding header "content-type" = "application/octet-stream"
2016/11/03 17:26:58 http2: Transport encoding header "user-agent" = "google-api-go-client/0.5"
2016/11/03 17:26:58 http2: Transport encoding header "content-length" = "8388608"
2016/11/03 17:26:58 http2: Transport encoding header "accept-encoding" = "gzip"
2016/11/03 17:26:58 http2: Framer 0xc4201dd380: wrote HEADERS flags=END_HEADERS stream=5 len=199
bradfitz commented 8 years ago

Hey @Capstan, can you shed any light on what we're supposed to be doing here? I've never seen a 308 without a Location header. Is that intentional from GCS's side?

Capstan commented 8 years ago

I would have thought the UploadServer would always yield the location header. I can get this into the hands of that team. The response type is described at 308 Resume Incomplete. The Location header in this case would just be the same as the header to the method, since you'd need to continue the upload. This cannot be handled with a raw redirect-follow.

bradfitz commented 8 years ago

Ugh, right. Thanks for the reminder about GCS's 308.

Yeah, we support GCS resumable uploads and such. But previous Go would ignore 308 responses. As of Go 1.8, Go will have support for 307 and 308s (including replaying the request body), which meant Go started handling UploadServer's 308 response as a redirect, instead of the "308 Resume Incomplete" semantics that GCS apparently invented as an alternate meaning for 308.

I think I'll just change the Go storage client to use http.Transport.RoundTrip instead of http.Client.Do, the only major difference being who handles 3xx.

bradfitz commented 8 years ago

But maybe we should also modify the Go http client to just stop and return the 308 response without an error if it lacks the Location header. That would at least match the behavior in previous Go releases.

gopherbot commented 8 years ago

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

bradfitz commented 8 years ago

Two separate fixes, both of which solve the problem on their own:

The second fix (now submitted) is more specific. The first one is a nice safety net, and will help people who update Go but not their google-api-go-client code.