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

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/16272)
<!-- Reviewable:end -->

components/net/fetch/methods.rs

@@ -163,15 +163,8 @@ pub fn main_fetch(request: &mut Request,
     // TODO: handle FTP URLs.
 
     // Step 10.
-    if !request.current_url().is_secure_scheme() && request.current_url().domain().is_some() {
+    context.state.hsts_list.read().unwrap().switch_known_hsts_host_domain_url_to_https(
-        if context.state
+        request.current_url_mut());
-            .hsts_list
-            .read()
-            .unwrap()
-            .is_host_secure(request.current_url().domain().unwrap()) {
-           request.url_list.last_mut().unwrap().as_mut_url().set_scheme("https").unwrap();
-        }
-    }
 
     // Step 11.
     // Not applicable: see fetch_async.
@@ -497,11 +490,6 @@ pub fn is_simple_method(m: &Method) -> bool {
     }
 }
 
-// fn modify_request_headers(headers: &mut Headers) -> {
-//     // TODO this function
-
-// }
-
 fn is_null_body_status(status: &Option<StatusCode>) -> bool {
     match *status {
         Some(status) => match status {

components/net/hsts.rs

@@ -6,11 +6,11 @@ use net_traits::IncludeSubdomains;
 use net_traits::pub_domains::reg_suffix;
 use serde_json;
 use servo_config::resource_files::read_resource_file;
+use servo_url::ServoUrl;
 use std::collections::HashMap;
 use std::net::{Ipv4Addr, Ipv6Addr};
 use std::str::from_utf8;
 use time;
-use url::Url;
 
 #[derive(Clone, Deserialize, Serialize)]
 pub struct HstsEntry {
@@ -135,16 +135,14 @@ impl HstsList {
             }
         }
     }
-}
 
-pub fn secure_url(url: &Url) -> Url {
+    /// Step 10 of https://fetch.spec.whatwg.org/#concept-main-fetch.
-    if url.scheme() == "http" {
+    pub fn switch_known_hsts_host_domain_url_to_https(&self, url: &mut ServoUrl) {
-        let mut secure_url = url.clone();
+        if url.scheme() != "http" {
-        secure_url.set_scheme("https").unwrap();
+            return;
-        // .set_port(Some(443)) would set the port to None,
+        }
-        // and should only be done when it was already None.
+        if url.domain().map_or(false, |domain| self.is_host_secure(domain)) {
-        secure_url
+            url.as_mut_url().set_scheme("https").unwrap();
-    } else {
+        }
-        url.clone()
     }
 }

components/net/websocket_loader.rs

@@ -36,7 +36,7 @@ pub fn init(connect: WebSocketCommunicate,
             connect_data: WebSocketConnectData,
             http_state: Arc<HttpState>) {
     thread::Builder::new().name(format!("WebSocket connection to {}", connect_data.resource_url)).spawn(move || {
-        let channel = establish_a_websocket_connection(&connect_data.resource_url,
+        let channel = establish_a_websocket_connection(connect_data.resource_url,
                                                        connect_data.origin,
                                                        connect_data.protocols,
                                                        &http_state);
@@ -146,7 +146,7 @@ fn obtain_a_websocket_connection(url: &ServoUrl) -> Result<Stream, NetworkError>
 }
 
 // https://fetch.spec.whatwg.org/#concept-websocket-establish
-fn establish_a_websocket_connection(resource_url: &ServoUrl,
+fn establish_a_websocket_connection(resource_url: ServoUrl,
                                     origin: String,
                                     protocols: Vec<String>,
                                     http_state: &HttpState)
@@ -268,7 +268,7 @@ struct Response {
 }
 
 // https://fetch.spec.whatwg.org/#concept-fetch
-fn fetch(url: &ServoUrl,
+fn fetch(url: ServoUrl,
          origin: String,
          mut headers: Headers,
          http_state: &HttpState)
@@ -306,7 +306,7 @@ fn fetch(url: &ServoUrl,
 }
 
 // https://fetch.spec.whatwg.org/#concept-main-fetch
-fn main_fetch(url: &ServoUrl,
+fn main_fetch(url: ServoUrl,
               origin: String,
               mut headers: Headers,
               http_state: &HttpState)
@@ -324,7 +324,7 @@ fn main_fetch(url: &ServoUrl,
     // TODO: handle upgrade to a potentially secure URL.
 
     // Step 5.
-    if should_be_blocked_due_to_bad_port(url) {
+    if should_be_blocked_due_to_bad_port(&url) {
         response = Some(Err(NetworkError::Internal("Request should be blocked due to bad port.".into())));
     }
     // TODO: handle blocking as mixed content.
@@ -352,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, http_state)
+        basic_fetch(&url, origin, &mut headers, http_state)
     });
 
     // Step 13.

components/net_traits/request.rs

@@ -186,45 +186,69 @@ impl Default for RequestInit {
     }
 }
 
-/// A [Request](https://fetch.spec.whatwg.org/#requests) as defined by the Fetch spec
+/// A [Request](https://fetch.spec.whatwg.org/#concept-request) as defined by
+/// the Fetch spec.
 #[derive(Clone, HeapSizeOf)]
 pub struct Request {
+    /// https://fetch.spec.whatwg.org/#concept-request-method
     #[ignore_heap_size_of = "Defined in hyper"]
     pub method: Method,
+    /// https://fetch.spec.whatwg.org/#local-urls-only-flag
     pub local_urls_only: bool,
+    /// https://fetch.spec.whatwg.org/#sandboxed-storage-area-urls-flag
     pub sandboxed_storage_area_urls: bool,
+    /// https://fetch.spec.whatwg.org/#concept-request-header-list
     #[ignore_heap_size_of = "Defined in hyper"]
     pub headers: Headers,
+    /// https://fetch.spec.whatwg.org/#unsafe-request-flag
     pub unsafe_request: bool,
+    /// https://fetch.spec.whatwg.org/#concept-request-body
     pub body: Option<Vec<u8>>,
     // TODO: client object
     pub is_service_worker_global_scope: bool,
     pub window: Window,
     // TODO: target browsing context
+    /// https://fetch.spec.whatwg.org/#request-keepalive-flag
     pub keep_alive: bool,
     pub skip_service_worker: bool,
+    /// https://fetch.spec.whatwg.org/#concept-request-initiator
     pub initiator: Initiator,
+    /// https://fetch.spec.whatwg.org/#concept-request-type
     pub type_: Type,
+    /// https://fetch.spec.whatwg.org/#concept-request-destination
     pub destination: Destination,
     // TODO: priority object
+    /// https://fetch.spec.whatwg.org/#concept-request-origin
     pub origin: Origin,
     pub omit_origin_header: bool,
     /// https://fetch.spec.whatwg.org/#concept-request-referrer
     pub referrer: Referrer,
+    /// https://fetch.spec.whatwg.org/#concept-request-referrer-policy
     pub referrer_policy: Option<ReferrerPolicy>,
     pub pipeline_id: Option<PipelineId>,
+    /// https://fetch.spec.whatwg.org/#synchronous-flag
     pub synchronous: bool,
+    /// https://fetch.spec.whatwg.org/#concept-request-mode
     pub mode: RequestMode,
+    /// https://fetch.spec.whatwg.org/#use-cors-preflight-flag
     pub use_cors_preflight: bool,
+    /// https://fetch.spec.whatwg.org/#concept-request-credentials-mode
     pub credentials_mode: CredentialsMode,
+    /// https://fetch.spec.whatwg.org/#concept-request-use-url-credentials-flag
     pub use_url_credentials: bool,
+    /// https://fetch.spec.whatwg.org/#concept-request-cache-mode
     pub cache_mode: CacheMode,
+    /// https://fetch.spec.whatwg.org/#concept-request-redirect-mode
     pub redirect_mode: RedirectMode,
+    /// https://fetch.spec.whatwg.org/#concept-request-integrity-metadata
     pub integrity_metadata: String,
     // Use the last method on url_list to act as spec current url field, and
     // first method to act as spec url field
+    /// https://fetch.spec.whatwg.org/#concept-request-url-list
     pub url_list: Vec<ServoUrl>,
+    /// https://fetch.spec.whatwg.org/#concept-request-redirect-count
     pub redirect_count: u32,
+    /// https://fetch.spec.whatwg.org/#concept-request-response-tainting
     pub response_tainting: ResponseTainting,
 }
 
@@ -296,18 +320,27 @@ impl Request {
         req
     }
 
+    /// https://fetch.spec.whatwg.org/#concept-request-url
     pub fn url(&self) -> ServoUrl {
         self.url_list.first().unwrap().clone()
     }
 
+    /// https://fetch.spec.whatwg.org/#concept-request-current-url
     pub fn current_url(&self) -> ServoUrl {
         self.url_list.last().unwrap().clone()
     }
 
+    /// https://fetch.spec.whatwg.org/#concept-request-current-url
+    pub fn current_url_mut(&mut self) -> &mut ServoUrl {
+        self.url_list.last_mut().unwrap()
+    }
+
+    /// https://fetch.spec.whatwg.org/#navigation-request
     pub fn is_navigation_request(&self) -> bool {
         self.destination == Destination::Document
     }
 
+    /// https://fetch.spec.whatwg.org/#subresource-request
     pub fn is_subresource_request(&self) -> bool {
         match self.destination {
             Destination::Font | Destination::Image | Destination::Manifest | Destination::Media |

tests/unit/net/hsts.rs

@@ -3,7 +3,6 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use net::hsts::{HstsEntry, HstsList};
-use net::hsts::secure_url;
 use net_traits::IncludeSubdomains;
 use std::collections::HashMap;
 use time;
@@ -294,27 +293,3 @@ fn test_preload_hsts_domains_well_formed() {
     let hsts_list = HstsList::from_servo_preload();
     assert!(!hsts_list.entries_map.is_empty());
 }
-
-#[test]
-fn test_secure_url_does_not_change_explicit_port() {
-    let url = Url::parse("http://mozilla.org:8080/").unwrap();
-    let secure = secure_url(&url);
-
-    assert!(secure.port().unwrap() == 8080u16);
-}
-
-#[test]
-fn test_secure_url_does_not_affect_non_http_schemas() {
-    let url = Url::parse("file://mozilla.org").unwrap();
-    let secure = secure_url(&url);
-
-    assert_eq!(secure.scheme(), "file");
-}
-
-#[test]
-fn test_secure_url_forces_an_http_host_in_list_to_https() {
-    let url = Url::parse("http://mozilla.org").unwrap();
-    let secure = secure_url(&url);
-
-    assert_eq!(secure.scheme(), "https");
-}