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

No automatic chunked Transfer Encoding when using Request.send #43

Closed
Deluvi opened this issue Mar 26, 2020 · 5 comments
Closed

No automatic chunked Transfer Encoding when using Request.send #43

Deluvi opened this issue Mar 26, 2020 · 5 comments

Comments

@Deluvi
Copy link
Contributor

Deluvi commented Mar 26, 2020

On ureq version 0.12.0:

When using the Request.send method, no headers are set to indicate some body content to the server. It causes some servers to completely ignore the body (like tiny_http).
The Request.send method should automatically enable the chunked encoding.

A workaround for this issue is to set the Transfer-Encoding header prior sending the request to enable the chunked transfer:

request.set("Transfer-Encoding", "chunked");
let response = request.send(body_reader);

I am doing this issue more as a warning for people like me that will stumble upon this gotcha. I have noticed that you are actually working a brand new version on ureq that probably fixed that issue already so it's maybe not a good idea to spend time fixing this issue. Maybe it should just be mentioned in the documentation of Request.send that the user may want to set the chunked transfer.

@algesten
Copy link
Owner

You're right! It could be good to set this header if we haven't set a content-length already. At this point it would be a (subtle) breaking change, so not entirely how to handle it.

Maybe submit a PR and we do it when we bump to 0.13.0?

I'm not rewriting ureq, it will stay the same (sync forever), and I will keep maintaining it until there's 0 interest in it. My rewrite efforts are in hreq.

@Deluvi
Copy link
Contributor Author

Deluvi commented Mar 28, 2020

Alright, thank you for your answer! I'll submit a PR when I have some time: I have a pretty good idea where to do the edit.

@jsha
Copy link
Collaborator

jsha commented Jun 20, 2020

Here's the relevant section of RFC 7230:

The presence of a message body in a request is signaled by a
Content-Length or Transfer-Encoding header field. Request message
framing is independent of method semantics, even if the method does
not define any use for a message body.

So I think tiny_http is right to ignore message bodies if neither Content-Length nor Transfer-Encoding is provided.

Right now there are two ways to provide a message body in ureq: .send_string() and .send(). If you want to send arbitrary binary content (i.e. not UTF-8), you need to use .send(). The user of ureq can currently choose between Content-Length (if they know the size of the data they'll send) or Transfer-Encoding (if they don't). It would be great to add some support to make it easier to do the right thing, but keep in mind making .send() always set Transfer-Encoding would effectively force chunked encoding for anyone who wants to send message bodies containing arbitrary bytes.

That would probably be okay, since the HTTP spec makes it mandatory to support chunked encoding:

A recipient MUST be able to parse the chunked transfer coding
(Section 4.1) because it plays a crucial role in framing messages
when the payload body size is not known in advance.

But that's definitely worth documenting. And if it turns out in the future that this causes compatibility problems, ureq could add a send_bytes(bytes: Vec<u8>) for arbitrary bytes of known size.

@jsha
Copy link
Collaborator

jsha commented Jun 20, 2020

Oops, I see that there already is a .send_bytes()! I was going off the documentation https://docs.rs/ureq/1.1.2/ureq/index.html#plain-requests. I've added that to my documentation PR #73, along with some notes on the current implementation of chunked encoding for .send().

Based on that updated understanding, I agree it makes sense to set Transfer-Encoding: chunked for the user when .send() is called. I also think it would be nice to error out if both Transfer-Encoding and Content-Length are set.

@algesten
Copy link
Owner

I believe this is working now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants