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(android): skip duplicated Content-Type/Content-Length header to fix tauri-apps/tauri#8857 #1360

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

Steve-xmh
Copy link
Contributor

As title follows, currently Android binding need to pass Content-Type separately when creating WebResourceReponse objects, but the response_headers doesn't skip Content-Type and Content-Length headers, which will be append into auto-generated headers with comma, which causes issue tauri-apps/tauri#8857 (e.g. application/wasm becomes application/wasm, application/wasm)

This PR fixed it by adding single conditional statement to skip these headers inside for loop when putting header into objects.

@Steve-xmh Steve-xmh requested a review from a team as a code owner September 10, 2024 11:12
Copy link
Contributor

github-actions bot commented Sep 10, 2024

Package Changes Through 87d8da5

There are 1 changes which include wry with patch

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
wry 0.43.1 0.43.2

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

Thank you, could you also add a change file in .changes directory?

@@ -216,6 +216,9 @@ fn handle_request(
let response_headers = {
let headers_map = JMap::from_env(env, &obj)?;
for (name, value) in headers.iter() {
if name == CONTENT_TYPE || name == CONTENT_LENGTH {
Copy link
Member

Choose a reason for hiding this comment

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

let's add a comment to explain why we are skipping these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's added now :D

@amrbashir
Copy link
Member

Thank you

@amrbashir amrbashir merged commit 170095b into tauri-apps:dev Sep 10, 2024
13 checks passed
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.

2 participants