Skip to content

Bump to latest release of conduit-hyper #2533

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

Merged
merged 1 commit into from
May 27, 2020

Conversation

jtgeibel
Copy link
Member

This upstream release now percent decodes the path component but not the
query string. This behavior aligns with the civet server. The known
difference is that conduit-hyper does a lossy utf8 conversion while
civet panics on invalid utf8 and closes the connection immediately.

This seems like a good compromise of matching the existing behavior as
closely as possible without copying the panic. I looked into fixing the
panic in civet, however the to_str_slice function returns an
Option<&str> and there is nowhere to store a newly allocated String
so changing the behavior there is not practical.

r? @JohnTitor
cc #2204

conduit-rust/conduit-hyper@v0.3.0-alpha.3...v0.3.0-alpha.4

This upstream release now percent decodes the path component but not the
query string. This behavior aligns with the `civet` server. The known
difference is that `conduit-hyper` does a lossy utf8 conversion while
`civet` panics on invalid utf8 and closes the connection immediately.

This seems like a good compromise of matching the existing behavior as
closely as possible without copying the panic. I looked into fixing the
panic in `civet`, however the `to_str_slice` function returns an
`Option<&str>` and there is nowhere to store a newly allocated `String`
so changing the behavior there is not practical.

conduit-rust/conduit-hyper@v0.3.0-alpha.3...v0.3.0-alpha.4
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Looks good!

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 27, 2020

📌 Commit 58bdab9 has been approved by JohnTitor

@bors
Copy link
Contributor

bors commented May 27, 2020

⌛ Testing commit 58bdab9 with merge 2df7413...

@bors
Copy link
Contributor

bors commented May 27, 2020

☀️ Test successful - checks-travis
Approved by: JohnTitor
Pushing 2df7413 to master...

@bors bors merged commit 2df7413 into rust-lang:master May 27, 2020
@jtgeibel jtgeibel deleted the percent-decode-uri-path branch September 20, 2020 04:06
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.

4 participants