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

The size_hint in HttpBody can change behavior when making GET requests with bodies #2444

Open
aturon opened this issue Feb 24, 2021 · 5 comments
Labels
A-body Area: body streaming.

Comments

@aturon
Copy link
Contributor

aturon commented Feb 24, 2021

This is a subtle one! Bodies for GET requests are ignored, unless the HttpBody::size_hint provides an exact size, in which case they are transmitted. The size_hint should not affect behavior in this way, AIUI.

I was using the general Client::request API to pass through a request that happened to be a GET with a body.

@seanmonstar
Copy link
Member

Hmmm, so you have a custom type implementing size_hint that returns a possible range instead of an exact size, is that right? It does seem that if you declare there is some size hint, it should mean the body should be sent with a transfer-encoding: chunked header. Is that what you'd expect as well?

@aturon
Copy link
Contributor Author

aturon commented Feb 24, 2021

I have a custom type that, depending on the situation, may return the default size hint (unbounded), or else an exact size. My surprise was that for a GET request, the size hint affected whether the body was sent at all. I would expect that if Hyper drops the body when making GET requests, it would so so in all cases, regardless of the size hint for the body.

@seanmonstar
Copy link
Member

I see, I'm starting to remember now. So, while very rare, it's legal to send a body with a GET. So hyper settled on these rules, currently:

  • If there's a content-length or transfer-encoding header explicitly, that wins.
  • If the body has an exact size, we'll send a content-length.
  • Otherwise, we'll assume you didn't mean to implicitly send a transfer-encoding body, which would possibly be empty and thus the server will find it weird.

@aturon
Copy link
Contributor Author

aturon commented Feb 25, 2021

Roger that! Thanks for the clarification. If this is intended behavior, I wonder if it could be added to documentation, in case others trip over the same issue.

@jeddenlea
Copy link

I think this is subtly related to #2427

@davidpdrsn davidpdrsn added the A-body Area: body streaming. label May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-body Area: body streaming.
Projects
None yet
Development

No branches or pull requests

4 participants