Skip to content

Commit

Permalink
Filters sensitive headers when redirecting to a Location of different…
Browse files Browse the repository at this point in the history
… host than of the Referrer

Removes Cookie, Authorization and WWW-Authenticate cookies.

Resolves seanmonstar#10
  • Loading branch information
steverob committed May 15, 2017
1 parent cd64041 commit 817f6c0
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 4 deletions.
7 changes: 3 additions & 4 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use serde_json;
use serde_urlencoded;

use ::body::{self, Body};
use ::redirect::{self, RedirectPolicy, check_redirect};
use ::redirect::{self, RedirectPolicy, check_redirect, remove_sensitive_headers};
use ::response::Response;

static DEFAULT_USER_AGENT: &'static str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"));
Expand Down Expand Up @@ -139,7 +139,6 @@ fn new_hyper_client() -> ::Result<::hyper::Client> {
))
}


/// A builder to construct the properties of a `Request`.
pub struct RequestBuilder {
client: Arc<ClientRef>,
Expand Down Expand Up @@ -313,6 +312,7 @@ impl RequestBuilder {
headers.set(Referer(url.to_string()));
urls.push(url);
let action = check_redirect(&client.redirect_policy.lock().unwrap(), &loc, &urls);

match action {
redirect::Action::Follow => loc,
redirect::Action::Stop => {
Expand All @@ -334,9 +334,8 @@ impl RequestBuilder {
}
};

headers = remove_sensitive_headers(headers, &url, &urls);
debug!("redirecting to {:?} '{}'", method, url);

//TODO: removeSensitiveHeaders(&mut headers, &url);
} else {
return Ok(::response::new(res, client.auto_ungzip.load(Ordering::Relaxed)))
}
Expand Down
37 changes: 37 additions & 0 deletions src/redirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ use std::fmt;

use ::Url;

#[allow(unused_imports)]
use hyper::header::{Headers, Authorization, Cookie, Accept};

/// A type that controls the policy on how to handle the following of redirects.
///
/// The default value will catch redirect loops, and has a maximum of 10
Expand Down Expand Up @@ -179,6 +182,16 @@ pub fn check_redirect(policy: &RedirectPolicy, next: &Url, previous: &[Url]) ->
}).inner
}

pub fn remove_sensitive_headers(mut headers: Headers, next: &Url, previous: &[Url]) -> Headers {
let cross_host = next.host().unwrap() != previous.last().unwrap().host().unwrap();
if cross_host {
headers.remove::<Authorization<String>>();
headers.remove::<Cookie>();
headers.remove_raw("www-authenticate");
}
headers
}

/*
This was the desired way of doing it, but ran in to inference issues when
using closures, since the arguments received are references (&Url and &[Url]),
Expand Down Expand Up @@ -229,3 +242,27 @@ fn test_redirect_policy_custom() {
let next = Url::parse("http://foo/baz").unwrap();
assert_eq!(check_redirect(&policy, &next, &[]), Action::Stop);
}

#[test]
fn test_remove_sensitive_headers() {
let mut headers = Headers::new();
headers.set(Accept::star());
headers.set(Authorization("let me in".to_owned()));
headers.set(
Cookie(vec![
String::from("foo=bar")
])
);

let next = Url::parse("http://initial-domain.com/path").unwrap();
let mut prev = vec![Url::parse("http://initial-domain.com/new_path").unwrap()];

assert_eq!(remove_sensitive_headers(headers.clone(), &next, &prev), headers);

prev.push(Url::parse("http://new-domain.com/path").unwrap());
let mut filtered_headers = headers.clone();
filtered_headers.remove::<Authorization<String>>();
filtered_headers.remove::<Cookie>();

assert_eq!(remove_sensitive_headers(headers.clone(), &next, &prev), filtered_headers);
}

0 comments on commit 817f6c0

Please sign in to comment.