-
Notifications
You must be signed in to change notification settings - Fork 174
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
http_interop: Remove requirement for header values to be UTF-8 #672
http_interop: Remove requirement for header values to be UTF-8 #672
Conversation
11189ad
to
b46a442
Compare
b46a442
to
1d1d3a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Can we construct a test for this? |
1d1d3a8
to
485656a
Compare
Sure! I added them for requests but probably also have to add them for responses... And found that the current |
bcb160a
to
c354e58
Compare
Looks great! Thanks! Maybe that test helper function need to be allowed unused for some test feature flag combo? |
c354e58
to
9171db5
Compare
Yeah. It's a bit nasty though as it's a useful function, just not something to expose publicly and the conditional is quite large now... |
9171db5
to
458d80e
Compare
…ngly) skipped during conversion
The current interop implementation forces fallibility around header values when they are not UTF-8, both in conversions from `http` to `ureq` as well as the other way around. This should not be necessary as both `http` and `ureq` treat these as opaque byte arrays internally, but unfortunately the current open API for `ureq` and `http` make it a bit nasty to deal with.
458d80e
to
a857d88
Compare
Thank you! This looks great! |
@algesten thanks! This should be the last of my PRs to |
CC @kade-robertson
The current interop implementation forces fallibility around header values when they are not UTF-8, both in conversions from
http
toureq
as well as the other way around.This should not be necessary as both
http
andureq
treat these as opaque byte arrays internally, but unfortunately the current open API forureq
andhttp
make it a bit nasty to deal with.