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.

Sign up for GitHub

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
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 42 additions & 18 deletions src/isahc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,49 @@ impl IsahcClient {
#[async_trait]
impl HttpClient for IsahcClient {
async fn send(&self, mut req: Request) -> Result<Response, Error> {
let mut builder = http::Request::builder()
.uri(req.url().as_str())
.method(http::Method::from_bytes(req.method().to_string().as_bytes()).unwrap());

for (name, value) in req.iter() {
builder = builder.header(name.as_str(), value.as_str());
}

let body = req.take_body();
let body = match body.len() {
Some(len) => isahc::Body::from_reader_sized(body, len as u64),
None => isahc::Body::from_reader(body),
};

let request = builder.body(body).unwrap();
let res = self.client.send_async(request).await.map_err(Error::from)?;
let maybe_metrics = res.metrics().cloned();
let (parts, body) = res.into_parts();
let body = Body::from_reader(BufReader::new(body), None);
// Code duplication to get around https://github.com/http-rs/surf/issues/321
// This is because `http::request` is generic (sized) over an existent and non-existent body differently, which is less than ideal.
let (parts, response_body, maybe_metrics) = match body.len() {
Some(len) => {
let mut builder = http::Request::builder()
.uri(req.url().as_str())
.method(http::Method::from_bytes(req.method().to_string().as_bytes()).unwrap());

for (name, value) in req.iter() {
builder = builder.header(name.as_str(), value.as_str());
}
let body = isahc::Body::from_reader_sized(body, len as u64);
let request = builder.body(body).unwrap();
let res = self.client.send_async(request).await.map_err(Error::from)?;
let maybe_metrics = res.metrics().cloned();
let (parts, body) = res.into_parts();
(
parts,
Body::from_reader(BufReader::new(body), None),
maybe_metrics,
)
}
None => {
let mut builder = http::Request::builder()
.uri(req.url().as_str())
.method(http::Method::from_bytes(req.method().to_string().as_bytes()).unwrap());

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>.

let res = self.client.send_async(request).await.map_err(Error::from)?;
let maybe_metrics = res.metrics().cloned();
let (parts, body) = res.into_parts();
(
parts,
Body::from_reader(BufReader::new(body), None),
maybe_metrics,
)
}
};
let mut response = http_types::Response::new(parts.status.as_u16());
for (name, value) in &parts.headers {
response.append_header(name.as_str(), value.to_str().unwrap());
Expand All @@ -69,7 +93,7 @@ impl HttpClient for IsahcClient {
response.ext_mut().insert(metrics);
}

response.set_body(body);
response.set_body(response_body);
Ok(response)
}

Expand Down