Skip to content
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: client `Expect: 100-continue` reads first byte of input on failure #16002

Closed
ncw opened this issue Jun 8, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@ncw
Copy link
Contributor

commented Jun 8, 2016

I'm trying to use Expect: 100-continue from the net/http client so that I can retry (after refreshing the token) requests with a body when I get a 401 request.

Do do this I'm passing in an io.Reader into http.NewRequest and setting the header thus

req, err := http.NewRequest("PUT", url, body)
// err handling
req.Header.Set("Expect", "100-continue")

However what I find is that 1 byte gets read from the reader even if the request fails with a 401 error.

This means that I can't retry the request as 1 byte is now missing from the reader.

It might be that this is working as intended, in which case this behavior should be documented.

Playground demonstration

What version of Go are you using (go version)?

Fails on stable and tip

go version devel +3c6b668 Thu Jun 2 00:22:03 2016 +0000 linux/amd64
go version go1.6.2 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ncw/Code/Go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT="1"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

What did you do?

See https://play.golang.org/p/3jXlWyrgsa for example program

What did you expect to see?

No bytes read from reader on 401 error

What did you see instead?

1 bytes read from the reader

2009/11/10 23:00:01 Starting fetch with req = &{Method:PUT URL:http://127.0.0.1:18398/ Proto:HTTP/1.1 ProtoMajor:1 ProtoMinor:1 Header:map[Expect:[100-continue]] Body:{Reader:0x10779700} ContentLength:0 TransferEncoding:[] Close:false Host:127.0.0.1:18398 Form:map[] PostForm:map[] MultipartForm:<nil> Trailer:map[] RemoteAddr: RequestURI: TLS:<nil> Cancel:<nil>}
2009/11/10 23:00:01 reader.Read(1)
2009/11/10 23:00:01  -> 1 bytes
2009/11/10 23:00:01 Got PUT request with headers map[User-Agent:[Go-http-client/1.1] Expect:[100-continue] Accept-Encoding:[gzip]]
2009/11/10 23:00:01 Got expected 401 error
2009/11/10 23:00:01 reader.Read(512)
2009/11/10 23:00:01  -> 22 bytes
2009/11/10 23:00:01 reader.Read(1514)
2009/11/10 23:00:01  -> io.EOF
2009/11/10 23:00:01 Remains of body "ody should not be read"
2009/11/10 23:00:01 shouldn't have read body, but read 1 bytes
@yoshiyaka

This comment has been minimized.

Copy link

commented Sep 16, 2016

It might be worth noting that the playground example works as expected if TransferEncoding: []string{"chunked"} is set for the request: https://play.golang.org/p/bIhgbXWHGk

@bradfitz

This comment has been minimized.

Copy link
Member

commented Sep 16, 2016

I don't think it is entirely documentation, so removing the label.

I'd like to address a bunch of the redirect-related bugs in 1.8, and this falls under that.

@bradfitz bradfitz removed the Documentation label Sep 16, 2016

@gopherbot

This comment has been minimized.

Copy link

commented Sep 30, 2016

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

gopherbot pushed a commit that referenced this issue Sep 30, 2016

net/http: refactor testing of Request.Body on 0 ContentLength
Code movement only, to look more like the equivalent http2 code, and
to make an upcoming fix look more obvious.

Updates #16002 (to be fixed once this code is in)

Change-Id: Iaa4f965be14e98f9996e7c4624afe6e19bed1a80
Reviewed-on: https://go-review.googlesource.com/30087
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
@gopherbot

This comment has been minimized.

Copy link

commented Sep 30, 2016

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

@gopherbot

This comment has been minimized.

Copy link

commented Sep 30, 2016

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

gopherbot pushed a commit to golang/net that referenced this issue Oct 1, 2016

http2: don't sniff Request.Body on 100-continue requests in Transport
Tests in net/http: TestNoSniffExpectRequestBody_h2

Updates golang/go#16002

Change-Id: Ib5255c14f0a8fdafd36077b86442dff213815567
Reviewed-on: https://go-review.googlesource.com/30150
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

@gopherbot gopherbot closed this in 09fb795 Oct 1, 2016

@golang golang locked and limited conversation to collaborators Oct 1, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.