Fix overwriting of appended request headers (#493)

* don't overwrite appended user headers

* Fixes tests, all header ordering related

* does not need to clone default headers, added a test

src/async_impl/client.rs

@@ -6,6 +6,7 @@ use std::net::IpAddr;
 use bytes::Bytes;
 use futures::{Async, Future, Poll};
 use header::{
+    Entry,
     HeaderMap,
     HeaderValue,
     ACCEPT,
@@ -526,13 +527,16 @@ impl Client {
         let (
             method,
             url,
-            user_headers,
+            mut headers,
             body
         ) = req.pieces();
 
-        let mut headers = self.inner.headers.clone(); // default headers
+        // insert default headers in the request headers
-        for (key, value) in user_headers.iter() {
+        // without overwriting already appended headers.
-            headers.insert(key, value.clone());
+        for (key, value) in &self.inner.headers {
+            if let Ok(Entry::Vacant(entry)) = headers.entry(key) {
+                entry.insert(value.clone());
+            }
         }
 
         // Add cookies from the cookie store.

tests/async.rs

@@ -59,9 +59,9 @@ fn async_test_multipart() {
     let server = server! {
         request: format!("\
             POST /multipart/1 HTTP/1.1\r\n\
+            content-type: multipart/form-data; boundary={}\r\n\
             user-agent: $USERAGENT\r\n\
             accept: */*\r\n\
-            content-type: multipart/form-data; boundary={}\r\n\
             accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             transfer-encoding: chunked\r\n\

tests/client.rs

@@ -139,9 +139,9 @@ fn test_post() {
     let server = server! {
         request: b"\
             POST /2 HTTP/1.1\r\n\
+            content-length: 5\r\n\
             user-agent: $USERAGENT\r\n\
             accept: */*\r\n\
-            content-length: 5\r\n\
             accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             \r\n\
@@ -177,10 +177,10 @@ fn test_post_form() {
     let server = server! {
         request: b"\
             POST /form HTTP/1.1\r\n\
-            user-agent: $USERAGENT\r\n\
-            accept: */*\r\n\
             content-type: application/x-www-form-urlencoded\r\n\
             content-length: 24\r\n\
+            user-agent: $USERAGENT\r\n\
+            accept: */*\r\n\
             accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             \r\n\
@@ -339,9 +339,9 @@ fn test_override_default_headers() {
     let server = server! {
         request: b"\
             GET /3 HTTP/1.1\r\n\
+            authorization: secret\r\n\
             user-agent: $USERAGENT\r\n\
             accept: */*\r\n\
-            authorization: secret\r\n\
             accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             \r\n\
@@ -363,3 +363,68 @@ fn test_override_default_headers() {
     assert_eq!(res.headers().get(reqwest::header::CONTENT_LENGTH).unwrap(), &"0");
 
 }
+
+#[test]
+fn test_appended_headers_not_overwritten() {
+    let client = reqwest::Client::new();
+
+    let server = server! {
+        request: b"\
+            GET /4 HTTP/1.1\r\n\
+            accept: application/json\r\n\
+            accept: application/json+hal\r\n\
+            user-agent: $USERAGENT\r\n\
+            accept-encoding: gzip\r\n\
+            host: $HOST\r\n\
+            \r\n\
+            ",
+        response: b"\
+            HTTP/1.1 200 OK\r\n\
+            Server: test\r\n\
+            Content-Length: 0\r\n\
+            \r\n\
+            "
+    };
+
+    let url = format!("http://{}/4", server.addr());
+    let res = client.get(&url).header(header::ACCEPT, "application/json").header(header::ACCEPT, "application/json+hal").send().unwrap();
+
+    assert_eq!(res.url().as_str(), &url);
+    assert_eq!(res.status(), reqwest::StatusCode::OK);
+    assert_eq!(res.headers().get(reqwest::header::SERVER).unwrap(), &"test");
+    assert_eq!(res.headers().get(reqwest::header::CONTENT_LENGTH).unwrap(), &"0");
+
+    // make sure this also works with default headers
+    use reqwest::header;
+    let mut headers = header::HeaderMap::with_capacity(1);
+    headers.insert(header::ACCEPT, header::HeaderValue::from_static("text/html"));
+    let client = reqwest::Client::builder()
+        .default_headers(headers)
+        .build().unwrap();
+    
+    let server = server! {
+        request: b"\
+            GET /4 HTTP/1.1\r\n\
+            accept: application/json\r\n\
+            accept: application/json+hal\r\n\
+            user-agent: $USERAGENT\r\n\
+            accept-encoding: gzip\r\n\
+            host: $HOST\r\n\
+            \r\n\
+            ",
+        response: b"\
+            HTTP/1.1 200 OK\r\n\
+            Server: test\r\n\
+            Content-Length: 0\r\n\
+            \r\n\
+            "
+    };
+
+    let url = format!("http://{}/4", server.addr());
+    let res = client.get(&url).header(header::ACCEPT, "application/json").header(header::ACCEPT, "application/json+hal").send().unwrap();
+
+    assert_eq!(res.url().as_str(), &url);
+    assert_eq!(res.status(), reqwest::StatusCode::OK);
+    assert_eq!(res.headers().get(reqwest::header::SERVER).unwrap(), &"test");
+    assert_eq!(res.headers().get(reqwest::header::CONTENT_LENGTH).unwrap(), &"0");
+}

tests/gzip.rs

@@ -113,8 +113,8 @@ fn test_accept_header_is_not_changed_if_set() {
     let server = server! {
         request: b"\
             GET /accept HTTP/1.1\r\n\
-            user-agent: $USERAGENT\r\n\
             accept: application/json\r\n\
+            user-agent: $USERAGENT\r\n\
             accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             \r\n\
@@ -142,9 +142,9 @@ fn test_accept_encoding_header_is_not_changed_if_set() {
     let server = server! {
         request: b"\
             GET /accept-encoding HTTP/1.1\r\n\
+            accept-encoding: identity\r\n\
             user-agent: $USERAGENT\r\n\
             accept: */*\r\n\
-            accept-encoding: identity\r\n\
             host: $HOST\r\n\
             \r\n\
             ",

tests/multipart.rs

@@ -21,10 +21,10 @@ fn text_part() {
     let server = server! {
         request: format!("\
             POST /multipart/1 HTTP/1.1\r\n\
-            user-agent: $USERAGENT\r\n\
-            accept: */*\r\n\
             content-type: multipart/form-data; boundary={}\r\n\
             content-length: 125\r\n\
+            user-agent: $USERAGENT\r\n\
+            accept: */*\r\n\
             accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             \r\n\
@@ -70,10 +70,10 @@ fn file() {
     let server = server! {
         request: format!("\
             POST /multipart/2 HTTP/1.1\r\n\
-            user-agent: $USERAGENT\r\n\
-            accept: */*\r\n\
             content-type: multipart/form-data; boundary={}\r\n\
             content-length: {}\r\n\
+            user-agent: $USERAGENT\r\n\
+            accept: */*\r\n\
             accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             \r\n\

tests/redirect.rs

@@ -116,9 +116,9 @@ fn test_redirect_307_and_308_tries_to_post_again() {
         let redirect = server! {
             request: format!("\
                 POST /{} HTTP/1.1\r\n\
+                content-length: 5\r\n\
                 user-agent: $USERAGENT\r\n\
                 accept: */*\r\n\
-                content-length: 5\r\n\
                 accept-encoding: gzip\r\n\
                 host: $HOST\r\n\
                 \r\n\
@@ -136,9 +136,9 @@ fn test_redirect_307_and_308_tries_to_post_again() {
 
             request: format!("\
                 POST /dst HTTP/1.1\r\n\
+                content-length: 5\r\n\
                 user-agent: $USERAGENT\r\n\
                 accept: */*\r\n\
-                content-length: 5\r\n\
                 accept-encoding: gzip\r\n\
                 referer: http://$HOST/{}\r\n\
                 host: $HOST\r\n\
@@ -211,9 +211,9 @@ fn test_redirect_removes_sensitive_headers() {
     let end_server = server! {
         request: b"\
             GET /otherhost HTTP/1.1\r\n\
+            accept-encoding: gzip\r\n\
             user-agent: $USERAGENT\r\n\
             accept: */*\r\n\
-            accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             \r\n\
             ",
@@ -228,9 +228,9 @@ fn test_redirect_removes_sensitive_headers() {
     let mid_server = server! {
         request: b"\
             GET /sensitive HTTP/1.1\r\n\
+            cookie: foo=bar\r\n\
             user-agent: $USERAGENT\r\n\
             accept: */*\r\n\
-            cookie: foo=bar\r\n\
             accept-encoding: gzip\r\n\
             host: $HOST\r\n\
             \r\n\