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

proposal: net/http: allow NewRequest to type assert on io.Readers with a Len method #30118

Closed
kortschak opened this issue Feb 7, 2019 · 2 comments
Closed

proposal: net/http: allow NewRequest to type assert on io.Readers with a Len method #30118

kortschak opened this issue Feb 7, 2019 · 2 comments
Labels
FrozenDueToAge Proposal
Milestone
Proposal

Comments

@kortschak
Copy link
Contributor

Currently NewRequest type switches on *bytes.Buffer, *bytes.Reader and *strings.Reader to determine whether the body's content length is known and so can be used to set the ContentLength header. It would be helpful in some cases for other io.Readers that are aware of their length to be able to be used in the same way by adding an interface { Len() int } case.

In AusOcean code we have seen two bugs arising from this since we send requests with an internal io.Reader that is not one of the three blessed types. For whatever reason, likely a bug in dev_appengine.py (during CI testing) this results in an error from the request that is avoided if the data is first copied into a *bytes.Buffer.

The addition of a interface { Len() int } case looks like it can not be used in conjunction with GetBody without use of reflect, though I'm not sure how significant this is.

See also https://groups.google.com/d/topic/golang-nuts/NdeMVAamx8E/discussion
@gopherbot gopherbot added this to the Proposal milestone Feb 7, 2019
@rsc rsc changed the title proposal: net/http: allow ` to type assert on io.Readers with a Len method proposal: net/http: allow NewRequest to type assert on io.Readers with a Len method Feb 13, 2019
@rsc
Copy link
Contributor

rsc commented Feb 13, 2019

It seems unfortunate to hard-code use of an interface that can't describe a 2GB file on 32-bit systems.

Note that you can set req.ContentLength in the result from NewRequest if you do know the size (or prepare the request yourself entirely; NewRequest is just a slight helper).

@rsc rsc closed this as completed Feb 13, 2019
@kortschak
Copy link
Contributor Author

Thanks, Russ. It's probably worth documenting the "workaround" more clearly since a few people have hit this. Careful thought leads to the correct approach, as noted in the linked thread, but often later than it should.

@kortschak kortschak mentioned this issue May 16, 2019
Closed
@golang golang locked and limited conversation to collaborators Feb 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
3 participants
@rsc @kortschak @gopherbot