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

Jump to bottom

net/http: client Expect: 100-continue reads first byte of input on failure #16002

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

net/http: client Expect: 100-continue reads first byte of input on failure #16002

ncw opened this issue Jun 8, 2016 · 5 comments
Labels
FrozenDueToAge
Milestone
Go1.8

Comments

@ncw
Copy link
Contributor

ncw 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
@ncw ncw mentioned this issue Jun 8, 2016
Closed
@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Jun 8, 2016
@ncw ncw mentioned this issue Jun 9, 2016
Open
@mr-andreas
Copy link

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
Copy link
Contributor

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.

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Sep 30, 2016
@bradfitz
net/http: refactor testing of Request.Body on 0 ContentLength
9491e7d 
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
Copy link

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

@gopherbot
Copy link

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

gopherbot pushed a commit to golang/net that referenced this issue Oct 1, 2016
@bradfitz
http2: don't sniff Request.Body on 100-continue requests in Transport
0d8126f 
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>
@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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
5 participants
@bradfitz @ncw @mr-andreas @ianlancetaylor @gopherbot