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
net/http: 308 response missing Location header #17773
Comments
Um, Google is actually replying like that:
And this was the request:
|
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? |
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. |
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 |
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. |
CL https://golang.org/cl/32595 mentions this issue. |
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. |
This CL tweaks the new (unreleased) 307/308 support added in https://golang.org/cl/29852 for #10767. Change 1: if a 307/308 response doesn't have a Location header in its response (as observed in the wild in #17773), just do what we used to do in Go 1.7 and earlier, and don't try to follow that redirect. Change 2: don't follow a 307/308 if we sent a body on the first request and the caller's Request.GetBody func is nil so we can't "rewind" the body to send it again. Updates #17773 (will be fixed more elsewhere) Change-Id: I183570f7346917828a4b6f7f1773094122a30406 Reviewed-on: https://go-review.googlesource.com/32595 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I just tried to upload a binary to Google Cloud Storage using golang.org/x/build/cmd/upload and got:
I suspect this is due to the recent 307/308 support in 7db996e (https://go-review.googlesource.com/29852).
/cc @odeke-em @jba
The text was updated successfully, but these errors were encountered: