Auto merge of #16256 - nox:net, r=jdm

Another batch of net enhancements 🕶

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16256)
<!-- Reviewable:end -->

components/net/fetch/methods.rs

@@ -8,12 +8,12 @@ use data_loader::decode;
 use devtools_traits::DevtoolsControlMsg;
 use fetch::cors_cache::CorsCache;
 use filemanager_thread::FileManager;
-use http_loader::{HttpState, determine_request_referrer, http_fetch, set_default_accept_language};
+use http_loader::{HttpState, determine_request_referrer, http_fetch};
+use http_loader::{set_default_accept, set_default_accept_language};
 use hyper::{Error, Result as HyperResult};
 use hyper::client::Pool;
 use hyper::header::{Accept, AcceptLanguage, ContentLanguage, ContentType};
-use hyper::header::{Header, HeaderFormat, HeaderView, Headers, QualityItem};
+use hyper::header::{Header, HeaderFormat, HeaderView, Headers, Referer as RefererHeader};
-use hyper::header::{Referer as RefererHeader, q, qitem};
 use hyper::method::Method;
 use hyper::mime::{Mime, SubLevel, TopLevel};
 use hyper::status::StatusCode;
@@ -61,64 +61,37 @@ pub fn fetch_with_cors_cache(request: &mut Request,
                              cache: &mut CorsCache,
                              target: Target,
                              context: &FetchContext) {
-    // Step 1
+    // Step 1.
     if request.window == Window::Client {
         // TODO: Set window to request's client object if client is a Window object
     } else {
         request.window = Window::NoWindow;
     }
 
-    // Step 2
+    // Step 2.
     if request.origin == Origin::Client {
         // TODO: set request's origin to request's client's origin
         unimplemented!()
     }
 
-    // Step 3
+    // Step 3.
-    if !request.headers.has::<Accept>() {
+    set_default_accept(request.type_, request.destination, &mut request.headers);
-        let value = match request.type_ {
-            // Substep 2
-            _ if request.is_navigation_request() =>
-                vec![qitem(mime!(Text / Html)),
-                     // FIXME: This should properly generate a MimeType that has a
-                     // SubLevel of xhtml+xml (https://github.com/hyperium/mime.rs/issues/22)
-                     qitem(mime!(Application / ("xhtml+xml") )),
-                     QualityItem::new(mime!(Application / Xml), q(0.9)),
-                     QualityItem::new(mime!(_ / _), q(0.8))],
 
-            // Substep 3
+    // Step 4.
-            Type::Image =>
-                vec![qitem(mime!(Image / Png)),
-                     // FIXME: This should properly generate a MimeType that has a
-                     // SubLevel of svg+xml (https://github.com/hyperium/mime.rs/issues/22)
-                     qitem(mime!(Image / ("svg+xml") )),
-                     QualityItem::new(mime!(Image / _), q(0.8)),
-                     QualityItem::new(mime!(_ / _), q(0.5))],
-
-            // Substep 3
-            Type::Style =>
-                vec![qitem(mime!(Text / Css)),
-                     QualityItem::new(mime!(_ / _), q(0.1))],
-            // Substep 1
-            _ => vec![qitem(mime!(_ / _))]
-        };
-
-        // Substep 4
-        request.headers.set(Accept(value));
-    }
-
-    // Step 4
     set_default_accept_language(&mut request.headers);
 
-    // Step 5
+    // Step 5.
-    // TODO: Figure out what a Priority object is
+    // TODO: figure out what a Priority object is.
 
-    // Step 6
+    // Step 6.
+    // TODO: handle client hints headers.
+
+    // Step 7.
     if request.is_subresource_request() {
-        // TODO: create a fetch record and append it to request's client's fetch group list
+        // TODO: handle client hints headers.
     }
 
-    // Step 7
+    // Step 8.
     main_fetch(request, cache, false, false, target, &mut None, &context);
 }
 

components/net/http_loader.rs

@@ -16,14 +16,16 @@ use hyper::Error as HttpError;
 use hyper::LanguageTag;
 use hyper::client::{Pool, Request as HyperRequest, Response as HyperResponse};
 use hyper::client::pool::PooledStream;
-use hyper::header::{AcceptEncoding, AcceptLanguage, AccessControlAllowCredentials};
+use hyper::header::{Accept, AccessControlAllowCredentials, AccessControlAllowHeaders};
-use hyper::header::{AccessControlAllowOrigin, AccessControlAllowHeaders, AccessControlAllowMethods};
+use hyper::header::{AccessControlAllowMethods, AccessControlAllowOrigin};
-use hyper::header::{AccessControlRequestHeaders, AccessControlMaxAge, AccessControlRequestMethod};
+use hyper::header::{AccessControlMaxAge, AccessControlRequestHeaders};
-use hyper::header::{Authorization, Basic, CacheControl, CacheDirective, ContentEncoding};
+use hyper::header::{AccessControlRequestMethod, AcceptEncoding, AcceptLanguage};
-use hyper::header::{ContentLength, Encoding, Header, Headers, Host, IfMatch, IfRange};
+use hyper::header::{Authorization, Basic, CacheControl, CacheDirective};
-use hyper::header::{IfUnmodifiedSince, IfModifiedSince, IfNoneMatch, Location, Pragma, Quality};
+use hyper::header::{ContentEncoding, ContentLength, Encoding, Header, Headers};
-use hyper::header::{QualityItem, Referer, SetCookie, UserAgent, qitem};
+use hyper::header::{Host, Origin as HyperOrigin, IfMatch, IfRange};
-use hyper::header::Origin as HyperOrigin;
+use hyper::header::{IfUnmodifiedSince, IfModifiedSince, IfNoneMatch, Location};
+use hyper::header::{Pragma, Quality, QualityItem, Referer, SetCookie};
+use hyper::header::{UserAgent, q, qitem};
 use hyper::method::Method;
 use hyper::net::{Fresh, HttpStream, HttpsStream, NetworkConnector};
 use hyper::status::StatusCode;
@@ -34,7 +36,8 @@ use msg::constellation_msg::PipelineId;
 use net_traits::{CookieSource, FetchMetadata, NetworkError, ReferrerPolicy};
 use net_traits::hosts::replace_host;
 use net_traits::request::{CacheMode, CredentialsMode, Destination, Origin};
-use net_traits::request::{RedirectMode, Referrer, Request, RequestMode, ResponseTainting};
+use net_traits::request::{RedirectMode, Referrer, Request, RequestMode};
+use net_traits::request::{ResponseTainting, Type};
 use net_traits::response::{HttpsState, Response, ResponseBody, ResponseType};
 use resource_thread::AuthCache;
 use servo_url::{ImmutableOrigin, ServoUrl};
@@ -141,6 +144,47 @@ impl NetworkHttpRequestFactory {
     }
 }
 
+// Step 3 of https://fetch.spec.whatwg.org/#concept-fetch.
+pub fn set_default_accept(type_: Type, destination: Destination, headers: &mut Headers) {
+    if headers.has::<Accept>() {
+        return;
+    }
+    let value = match (type_, destination) {
+        // Step 3.2.
+        (_, Destination::Document) => {
+            vec![
+                qitem(mime!(Text / Html)),
+                qitem(mime!(Application / ("xhtml+xml"))),
+                QualityItem::new(mime!(Application / Xml), q(0.9)),
+                QualityItem::new(mime!(_ / _), q(0.8)),
+            ]
+        },
+        // Step 3.3.
+        (Type::Image, _) => {
+            vec![
+                qitem(mime!(Image / Png)),
+                qitem(mime!(Image / ("svg+xml") )),
+                QualityItem::new(mime!(Image / _), q(0.8)),
+                QualityItem::new(mime!(_ / _), q(0.5)),
+            ]
+        },
+        // Step 3.3.
+        (Type::Style, _) => {
+            vec![
+                qitem(mime!(Text / Css)),
+                QualityItem::new(mime!(_ / _), q(0.1))
+            ]
+        },
+        // Step 3.1.
+        _ => {
+            vec![qitem(mime!(_ / _))]
+        },
+    };
+
+    // Step 3.4.
+    headers.set(Accept(value));
+}
+
 fn set_default_accept_encoding(headers: &mut Headers) {
     if headers.has::<AcceptEncoding>() {
         return

components/net/websocket_loader.rs

@@ -3,12 +3,11 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cookie::Cookie;
-use cookie_rs;
-use cookie_storage::CookieStorage;
 use fetch::methods::{should_be_blocked_due_to_bad_port, should_be_blocked_due_to_nosniff};
-use http_loader::{HttpState, is_redirect_status, set_request_cookies};
+use http_loader::{HttpState, is_redirect_status, set_default_accept};
+use http_loader::{set_default_accept_language, set_request_cookies};
 use hyper::buffer::BufReader;
-use hyper::header::{Accept, CacheControl, CacheDirective, Connection, ConnectionOption};
+use hyper::header::{CacheControl, CacheDirective, Connection, ConnectionOption};
 use hyper::header::{Headers, Host, SetCookie, Pragma, Protocol, ProtocolName, Upgrade};
 use hyper::http::h1::{LINE_ENDING, parse_response};
 use hyper::method::Method;
@@ -18,12 +17,12 @@ use hyper::version::HttpVersion;
 use net_traits::{CookieSource, MessageData, NetworkError, WebSocketCommunicate, WebSocketConnectData};
 use net_traits::{WebSocketDomAction, WebSocketNetworkEvent};
 use net_traits::hosts::replace_host;
-use net_traits::request::Type;
+use net_traits::request::{Destination, Type};
 use servo_url::ServoUrl;
 use std::ascii::AsciiExt;
 use std::io::{self, Write};
 use std::net::TcpStream;
-use std::sync::{Arc, Mutex, RwLock};
+use std::sync::{Arc, Mutex};
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::thread;
 use url::Position;
@@ -40,7 +39,7 @@ pub fn init(connect: WebSocketCommunicate,
         let channel = establish_a_websocket_connection(&connect_data.resource_url,
                                                        connect_data.origin,
                                                        connect_data.protocols,
-                                                       &http_state.cookie_jar);
+                                                       &http_state);
         let (ws_sender, mut receiver) = match channel {
             Ok((protocol_in_use, sender, receiver)) => {
                 let _ = connect.event_sender.send(WebSocketNetworkEvent::ConnectionEstablished { protocol_in_use });
@@ -150,7 +149,7 @@ fn obtain_a_websocket_connection(url: &ServoUrl) -> Result<Stream, NetworkError>
 fn establish_a_websocket_connection(resource_url: &ServoUrl,
                                     origin: String,
                                     protocols: Vec<String>,
-                                    cookie_jar: &RwLock<CookieStorage>)
+                                    http_state: &HttpState)
                                     -> Result<(Option<String>,
                                                Sender<Stream>,
                                                Receiver<Stream>),
@@ -185,7 +184,7 @@ fn establish_a_websocket_connection(resource_url: &ServoUrl,
     // TODO: handle permessage-deflate extension.
 
     // Step 11 and network error check from step 12.
-    let response = fetch(resource_url, origin, headers, cookie_jar)?;
+    let response = fetch(resource_url, origin, headers, http_state)?;
 
     // Step 12, the status code check.
     if response.status != StatusCode::SwitchingProtocols {
@@ -272,7 +271,7 @@ struct Response {
 fn fetch(url: &ServoUrl,
          origin: String,
          mut headers: Headers,
-         cookie_jar: &RwLock<CookieStorage>)
+         http_state: &HttpState)
          -> Result<Response, NetworkError> {
     // Step 1.
     // TODO: handle request's window.
@@ -281,23 +280,10 @@ fn fetch(url: &ServoUrl,
     // TODO: handle request's origin.
 
     // Step 3.
-    // We know there is no `Accept` header in `headers`.
+    set_default_accept(Type::None, Destination::None, &mut headers);
-    {
-        // Step 3.1.
-        let value = Accept::star();
-
-        // Step 3.2.
-        // Not applicable: not a navigation request.
-
-        // Step 3.3.
-        // Not applicable: request's type is the empty string.
-
-        // Step 3.4.
-        headers.set(value);
-    }
 
     // Step 4.
-    // TODO: handle `Accept-Language`.
+    set_default_accept_language(&mut headers);
 
     // Step 5.
     // TODO: handle request's priority.
@@ -316,14 +302,14 @@ fn fetch(url: &ServoUrl,
     }
 
     // Step 8.
-    main_fetch(url, origin, headers, cookie_jar)
+    main_fetch(url, origin, headers, http_state)
 }
 
 // https://fetch.spec.whatwg.org/#concept-main-fetch
 fn main_fetch(url: &ServoUrl,
               origin: String,
               mut headers: Headers,
-              cookie_jar: &RwLock<CookieStorage>)
+              http_state: &HttpState)
               -> Result<Response, NetworkError> {
     // Step 1.
     let mut response = None;
@@ -366,7 +352,7 @@ fn main_fetch(url: &ServoUrl,
         // doesn't need to be filtered at all.
 
         // Step 12.2.
-        basic_fetch(url, origin, &mut headers, cookie_jar)
+        basic_fetch(url, origin, &mut headers, http_state)
     });
 
     // Step 13.
@@ -404,17 +390,17 @@ fn main_fetch(url: &ServoUrl,
 fn basic_fetch(url: &ServoUrl,
                origin: String,
                headers: &mut Headers,
-               cookie_jar: &RwLock<CookieStorage>)
+               http_state: &HttpState)
                -> Result<Response, NetworkError> {
     // In the case of a WebSocket request, HTTP fetch is always used.
-    http_fetch(url, origin, headers, cookie_jar)
+    http_fetch(url, origin, headers, http_state)
 }
 
 // https://fetch.spec.whatwg.org/#concept-http-fetch
 fn http_fetch(url: &ServoUrl,
               origin: String,
               headers: &mut Headers,
-              cookie_jar: &RwLock<CookieStorage>)
+              http_state: &HttpState)
               -> Result<Response, NetworkError> {
     // Step 1.
     // Not applicable: with step 3 being useless here, this one is too.
@@ -435,7 +421,7 @@ fn http_fetch(url: &ServoUrl,
         // Not applicable: request's redirect mode is "error".
 
         // Step 4.3.
-        let response = http_network_or_cache_fetch(url, origin, headers, cookie_jar);
+        let response = http_network_or_cache_fetch(url, origin, headers, http_state);
 
         // Step 4.4.
         // Not applicable: CORS flag is unset.
@@ -464,7 +450,7 @@ fn http_fetch(url: &ServoUrl,
 fn http_network_or_cache_fetch(url: &ServoUrl,
                                origin: String,
                                headers: &mut Headers,
-                               cookie_jar: &RwLock<CookieStorage>)
+                               http_state: &HttpState)
                                -> Result<Response, NetworkError> {
     // Steps 1-3.
     // Not applicable: we don't even have a request yet, and there is no body
@@ -515,7 +501,7 @@ fn http_network_or_cache_fetch(url: &ServoUrl,
     {
         // Step 17.1.
         // TODO: handle user agent configured to block cookies.
-        set_request_cookies(&url, headers, &cookie_jar);
+        set_request_cookies(&url, headers, &http_state.cookie_jar);
 
         // Steps 17.2-6.
         // Not applicable: request has no Authorization header.
@@ -540,7 +526,7 @@ fn http_network_or_cache_fetch(url: &ServoUrl,
         // Not applicable: cache mode is "no-store".
 
         // Step 22.2.
-        let forward_response = http_network_fetch(url, headers, cookie_jar);
+        let forward_response = http_network_fetch(url, headers, http_state);
 
         // Step 22.3.
         // Not applicable: request's method is not unsafe.
@@ -569,7 +555,7 @@ fn http_network_or_cache_fetch(url: &ServoUrl,
 // https://fetch.spec.whatwg.org/#concept-http-network-fetch
 fn http_network_fetch(url: &ServoUrl,
                       headers: &Headers,
-                      cookie_jar: &RwLock<CookieStorage>)
+                      http_state: &HttpState)
                       -> Result<Response, NetworkError> {
     // Step 1.
     // Not applicable: credentials flag is set.
@@ -595,15 +581,13 @@ fn http_network_fetch(url: &ServoUrl,
 
     // Step 15.
     if let Some(cookies) = response.headers.get::<SetCookie>() {
-        let mut jar = cookie_jar.write().unwrap();
+        let mut jar = http_state.cookie_jar.write().unwrap();
         for cookie in &**cookies {
-            if let Ok(cookie) = cookie_rs::Cookie::parse(&**cookie) {
+            if let Some(cookie) = Cookie::from_cookie_string(cookie.clone(), url, CookieSource::HTTP) {
-                if let Some(cookie) = Cookie::new_wrapped(cookie.into_owned(), url, CookieSource::HTTP) {
                 jar.push(cookie, url, CookieSource::HTTP);
             }
         }
     }
-    }
 
     // Step 16.
     // Not applicable: correct WebSocket responses don't have a body.