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

Set negative value to second argument of esp_http_client_open method if request header has Transfer-Encoding: chunked #310

Merged
merged 4 commits into from
Oct 31, 2023
Merged

Set negative value to second argument of esp_http_client_open method if request header has Transfer-Encoding: chunked #310

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a86d5f7
Convert request_content_len to i32 because it would be casted to i32 …
Oct 30, 2023
0da0e59
Pass negative value if header has
Oct 30, 2023
adeaca5
Back request_content_len type to u64 and cast it to i32 when pass to …
Oct 30, 2023
a7d084c
Remove is_chunked_streaming_request and convert request_content_len f…
Oct 31, 2023
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
25 changes: 23 additions & 2 deletions src/http/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,16 @@ impl EspHttpConnection {

self.request_content_len = content_len.unwrap_or(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Assuming you'll change request_content_len to i64, as suggested below, just do here unwrap_or(-1)


esp!(unsafe { esp_http_client_open(self.raw_client, self.request_content_len as _) })?;
esp!(unsafe {
esp_http_client_open(
self.raw_client,
if self.is_chunked_streaming_request(headers) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the if. Assuming is_chunked_streaming_request is removed, and then unwrapping content-len just sets request_content_len to -1 if content_len is None, you'll no longer need it.

-1
} else {
self.request_content_len as i32
},
)
})?;

self.state = State::Request;

Expand Down Expand Up @@ -399,7 +408,7 @@ impl EspHttpConnection {
})?;
esp!(unsafe { esp_http_client_set_redirection(self.raw_client) })?;
esp!(unsafe {
esp_http_client_open(self.raw_client, self.request_content_len as _)
esp_http_client_open(self.raw_client, self.request_content_len as i32)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please just change the type of EspHttpConnection::request_content_len to i64, as you originally started doing. Sorry for asking you to do other stuff - I did not realize, that request_content_len is a private field which is not visible to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, change type to i64.
a7d084c

})?;

self.headers.clear();
Expand Down Expand Up @@ -442,6 +451,18 @@ impl EspHttpConnection {
panic!("connection is not in response phase");
}
}

Copy link
Collaborator

@ivmarkov ivmarkov Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think you don't need this method at all. This post says, that as long as you pass -1 to esp_http_client_open, the client would assume chunked encoding.

fn is_chunked_streaming_request(&self, headers: &[(&str, &str)]) -> bool {
for (name, value) in headers {
if name.eq_ignore_ascii_case("Transfer-Encoding")
&& value.eq_ignore_ascii_case("chunked")
{
return true;
}
}

false
}
}

impl Drop for EspHttpConnection {
Expand Down