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

fix: curl - avoid zero-sized HEAD bodies #97

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Fishrock123
Copy link
Member

If there is no body then don't construct a zero-lengthed body.

Fixes: http-rs/surf#321
Fixes: http-rs/surf#218

If there is no body then don't construct a zero-lengthed body.

Fixes: http-rs/surf#321
Fixes: http-rs/surf#218
@Fishrock123
Copy link
Member Author

Confirmed working: http-rs/surf#321 (comment)

@Fishrock123
Copy link
Member Author

@sagebind if you wouldn't mind, I'd like to know if this patch looks correct to you.

for (name, value) in req.iter() {
builder = builder.header(name.as_str(), value.as_str());
}
let request = builder.body(()).unwrap();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem right to me at all -- does Surf/http-client always know the size of a request body being uploaded? Because what this code seems to be doing is, "If we don't know the size of the request body, then just don't send one at all." What if you are streaming a request body of unknown size? I'd think that this change would break that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, from Isahc's perspective a body of () and a body of isahc::Body::empty() are identical, so you can use that instead so that you don't have to deal with the type shenanigans of dealing with two different cases of T in Request<T>.

@sagebind
Copy link

sagebind commented Sep 1, 2021

This doesn't seem to be aiming to correct the right thing here. The problem is that Isahc understands three different scenarios:

  • A request with a body of known size, including 0. (Body::from_reader_sized)
  • A request with a body of unknown size. (Body::from_reader)
  • A request without a body. (Body::empty, or equivalently ())

The problem is that Surf never indicates the third case to Isahc, only one of the first two cases.

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

Successfully merging this pull request may close these issues.

HEAD requests which return a content-length incorrectly Err method HEAD error
2 participants