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

RequestBuilder::header unsets sensitive on header value #2352

Closed
Ten0 opened this issue Jul 14, 2024 · 2 comments · Fixed by #2353
Closed

RequestBuilder::header unsets sensitive on header value #2352

Ten0 opened this issue Jul 14, 2024 · 2 comments · Fixed by #2353

Comments

@Ten0
Copy link
Contributor

Ten0 commented Jul 14, 2024

RequestBuilder::header:

pub fn header<K, V>(self, key: K, value: V) -> RequestBuilder
where
HeaderName: TryFrom<K>,
<HeaderName as TryFrom<K>>::Error: Into<http::Error>,
HeaderValue: TryFrom<V>,
<HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
{
self.header_sensitive(key, value, false)
}
/// Add a `Header` to this Request with ability to define if `header_value` is sensitive.
fn header_sensitive<K, V>(mut self, key: K, value: V, sensitive: bool) -> RequestBuilder
where
HeaderName: TryFrom<K>,
<HeaderName as TryFrom<K>>::Error: Into<http::Error>,
HeaderValue: TryFrom<V>,
<HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
{
let mut error = None;
if let Ok(ref mut req) = self.request {
match <HeaderName as TryFrom<K>>::try_from(key) {
Ok(key) => match <HeaderValue as TryFrom<V>>::try_from(value) {
Ok(mut value) => {
// We want to potentially make an unsensitive header
// to be sensitive, not the reverse. So, don't turn off
// a previously sensitive header.
if sensitive {
value.set_sensitive(true);
}
req.headers_mut().append(key, value);

allows passing HeaderValue, however it overwrites sensitive to false, making it impossible to flag a header as sensitive right away unless going through the .basic_auth or .bearer_auth functions.

It looks like a bug: when simply going through .header, it looks like it should not overwrite the value of sensitive for the generated HeaderValue.

@seanmonstar
Copy link
Owner

It does? Perhaps I'm not seeing it. Which line specifically does it unset it?

@Ten0
Copy link
Contributor Author

Ten0 commented Jul 14, 2024

Ah sorry I read the code for blocking in my editor, and actually linked the async one here.

The issue still lies here:

pub fn header<K, V>(self, key: K, value: V) -> RequestBuilder
where
HeaderName: TryFrom<K>,
HeaderValue: TryFrom<V>,
<HeaderName as TryFrom<K>>::Error: Into<http::Error>,
<HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
{
self.header_sensitive(key, value, false)
}
/// Add a `Header` to this Request with ability to define if header_value is sensitive.
fn header_sensitive<K, V>(mut self, key: K, value: V, sensitive: bool) -> RequestBuilder
where
HeaderName: TryFrom<K>,
HeaderValue: TryFrom<V>,
<HeaderName as TryFrom<K>>::Error: Into<http::Error>,
<HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
{
let mut error = None;
if let Ok(ref mut req) = self.request {
match <HeaderName as TryFrom<K>>::try_from(key) {
Ok(key) => match <HeaderValue as TryFrom<V>>::try_from(value) {
Ok(mut value) => {
value.set_sensitive(sensitive);
req.headers_mut().append(key, value);

It seems that the issue was already reported in #1549 and fixed for async by d536ce2, but the blocking implementation was forgotten.

I'll open a PR.

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 a pull request may close this issue.

2 participants