Auto merge of #25357 - Eijebong:redirects, r=jdm

Fix a few things regarding redirects

See individual commits

components/net/http_loader.rs

@@ -25,7 +25,10 @@ use headers::{
 use headers::{AccessControlAllowOrigin, AccessControlMaxAge};
 use headers::{CacheControl, ContentEncoding, ContentLength};
 use headers::{IfModifiedSince, LastModified, Origin as HyperOrigin, Pragma, Referer, UserAgent};
-use http::header::{self, HeaderName, HeaderValue};
+use http::header::{
+    self, HeaderName, HeaderValue, CONTENT_ENCODING, CONTENT_LANGUAGE, CONTENT_LOCATION,
+    CONTENT_TYPE,
+};
 use http::{HeaderMap, Request as HyperRequest};
 use hyper::{Body, Client, Method, Response as HyperResponse, StatusCode};
 use hyper_serde::Serde;
@@ -357,11 +360,9 @@ fn obtain_response(
     client: &Client<Connector, Body>,
     url: &ServoUrl,
     method: &Method,
-    request_headers: &HeaderMap,
+    headers: &HeaderMap,
     data: &Option<Vec<u8>>,
-    load_data_method: &Method,
     pipeline_id: &Option<PipelineId>,
-    iters: u32,
     request_id: Option<&str>,
     is_xhr: bool,
     context: &FetchContext,
@@ -371,28 +372,7 @@ fn obtain_response(
         Error = NetworkError,
     >,
 > {
-    let mut headers = request_headers.clone();
+    let request_body = data.as_ref().cloned().unwrap_or(vec![]);
-
-    // Avoid automatically sending request body if a redirect has occurred.
-    //
-    // TODO - This is the wrong behaviour according to the RFC. However, I'm not
-    // sure how much "correctness" vs. real-world is important in this case.
-    //
-    // https://tools.ietf.org/html/rfc7231#section-6.4
-    let is_redirected_request = iters != 1;
-    let request_body;
-    match data {
-        &Some(ref d) if !is_redirected_request => {
-            headers.typed_insert(ContentLength(d.len() as u64));
-            request_body = d.clone();
-        },
-        _ => {
-            if *load_data_method != Method::GET && *load_data_method != Method::HEAD {
-                headers.typed_insert(ContentLength(0))
-            }
-            request_body = vec![];
-        },
-    }
 
     context
         .timing
@@ -452,6 +432,7 @@ fn obtain_response(
     let method = method.clone();
     let send_start = precise_time_ms();
 
+    let headers = headers.clone();
     Box::new(
         client
             .request(request)
@@ -801,11 +782,23 @@ pub fn http_redirect_fetch(
         .map_or(false, |(code, _)| {
             ((*code == StatusCode::MOVED_PERMANENTLY || *code == StatusCode::FOUND) &&
                 request.method == Method::POST) ||
-                (*code == StatusCode::SEE_OTHER && request.method != Method::HEAD)
+                (*code == StatusCode::SEE_OTHER &&
+                    request.method != Method::HEAD &&
+                    request.method != Method::GET)
         })
     {
+        // Step 11.1
         request.method = Method::GET;
         request.body = None;
+        // Step 11.2
+        for name in &[
+            CONTENT_ENCODING,
+            CONTENT_LANGUAGE,
+            CONTENT_LOCATION,
+            CONTENT_TYPE,
+        ] {
+            request.headers.remove(name);
+        }
     }
 
     // Step 12
@@ -1398,9 +1391,7 @@ fn http_network_fetch(
         &request.method,
         &request.headers,
         &request.body,
-        &request.method,
         &request.pipeline_id,
-        request.redirect_count + 1,
         request_id.as_ref().map(Deref::deref),
         is_xhr,
         context,

components/net_traits/lib.rs

@@ -638,6 +638,8 @@ pub struct Metadata {
     pub referrer_policy: Option<ReferrerPolicy>,
     /// Performance information for navigation events
     pub timing: Option<ResourceFetchTiming>,
+    /// True if the request comes from a redirection
+    pub redirected: bool,
 }
 
 impl Metadata {
@@ -655,6 +657,7 @@ impl Metadata {
             referrer: None,
             referrer_policy: None,
             timing: None,
+            redirected: false,
         }
     }
 

components/net_traits/response.rs

@@ -310,6 +310,7 @@ impl Response {
             metadata.https_state = response.https_state;
             metadata.referrer = response.referrer.clone();
             metadata.referrer_policy = response.referrer_policy.clone();
+            metadata.redirected = response.actual_response().url_list.len() > 1;
             metadata
         };
 

components/script/dom/response.rs

@@ -51,6 +51,7 @@ pub struct Response {
     body_promise: DomRefCell<Option<(Rc<Promise>, BodyType)>>,
     #[ignore_malloc_size_of = "StreamConsumer"]
     stream_consumer: DomRefCell<Option<StreamConsumer>>,
+    redirected: DomRefCell<bool>,
 }
 
 #[allow(non_snake_case)]
@@ -69,6 +70,7 @@ impl Response {
             body: DomRefCell::new(NetTraitsResponseBody::Empty),
             body_promise: DomRefCell::new(None),
             stream_consumer: DomRefCell::new(None),
+            redirected: DomRefCell::new(false),
         }
     }
 
@@ -287,8 +289,7 @@ impl ResponseMethods for Response {
 
     // https://fetch.spec.whatwg.org/#dom-response-redirected
     fn Redirected(&self) -> bool {
-        let url_list_len = self.url_list.borrow().len();
+        return *self.redirected.borrow();
-        url_list_len > 1
     }
 
     // https://fetch.spec.whatwg.org/#dom-response-status
@@ -415,6 +416,10 @@ impl Response {
         *self.url.borrow_mut() = Some(final_url);
     }
 
+    pub fn set_redirected(&self, is_redirected: bool) {
+        *self.redirected.borrow_mut() = is_redirected;
+    }
+
     fn set_response_members_by_type(&self, response_type: DOMResponseType) {
         match response_type {
             DOMResponseType::Error => {

components/script/fetch.rs

@@ -306,6 +306,7 @@ fn fill_headers_with_metadata(r: DomRoot<Response>, m: Metadata) {
     r.set_headers(m.headers);
     r.set_raw_status(m.status);
     r.set_final_url(m.final_url);
+    r.set_redirected(m.redirected);
 }
 
 /// Convenience function for synchronously loading a whole resource.

tests/wpt/metadata/fetch/api/basic/request-headers.any.js.ini

@@ -3,14 +3,9 @@
   [Fetch with POST with FormData body]
     expected: FAIL
 
-  [Fetch with Chicken]
-    expected: FAIL
 
 [request-headers.any.worker.html]
   type: testharness
   [Fetch with POST with FormData body]
     expected: FAIL
 
-  [Fetch with Chicken]
-    expected: FAIL
-

tests/wpt/metadata/fetch/api/redirect/redirect-method.any.js.ini

@@ -1,87 +0,0 @@
-[redirect-method.any.worker.html]
-  [Redirect 301 with GET]
-    expected: FAIL
-
-  [Redirect 301 with POST]
-    expected: FAIL
-
-  [Redirect 301 with HEAD]
-    expected: FAIL
-
-  [Redirect 302 with GET]
-    expected: FAIL
-
-  [Redirect 302 with POST]
-    expected: FAIL
-
-  [Redirect 302 with HEAD]
-    expected: FAIL
-
-  [Redirect 303 with GET]
-    expected: FAIL
-
-  [Redirect 303 with POST]
-    expected: FAIL
-
-  [Redirect 303 with HEAD]
-    expected: FAIL
-
-  [Redirect 307 with GET]
-    expected: FAIL
-
-  [Redirect 307 with POST (string body)]
-    expected: FAIL
-
-  [Redirect 307 with POST (blob body)]
-    expected: FAIL
-
-  [Redirect 307 with HEAD]
-    expected: FAIL
-
-  [Redirect 303 with TESTING]
-    expected: FAIL
-
-
-[redirect-method.any.html]
-  [Redirect 301 with GET]
-    expected: FAIL
-
-  [Redirect 301 with POST]
-    expected: FAIL
-
-  [Redirect 301 with HEAD]
-    expected: FAIL
-
-  [Redirect 302 with GET]
-    expected: FAIL
-
-  [Redirect 302 with POST]
-    expected: FAIL
-
-  [Redirect 302 with HEAD]
-    expected: FAIL
-
-  [Redirect 303 with GET]
-    expected: FAIL
-
-  [Redirect 303 with POST]
-    expected: FAIL
-
-  [Redirect 303 with HEAD]
-    expected: FAIL
-
-  [Redirect 307 with GET]
-    expected: FAIL
-
-  [Redirect 307 with POST (string body)]
-    expected: FAIL
-
-  [Redirect 307 with POST (blob body)]
-    expected: FAIL
-
-  [Redirect 307 with HEAD]
-    expected: FAIL
-
-  [Redirect 303 with TESTING]
-    expected: FAIL
-

tests/wpt/metadata/xhr/send-redirect-post-upload.htm.ini

@@ -1,20 +1,5 @@
 [send-redirect-post-upload.htm]
   type: testharness
-  [XMLHttpRequest: The send() method: POSTing to URL that redirects (301)]
-    expected: FAIL
-
-  [XMLHttpRequest: The send() method: POSTing to URL that redirects (302)]
-    expected: FAIL
-
-  [XMLHttpRequest: The send() method: POSTing to URL that redirects (303)]
-    expected: FAIL
-
   [XMLHttpRequest: The send() method: POSTing to URL that redirects (307)]
     expected: FAIL
 
-  [XMLHttpRequest: The send() method: POSTing to URL that redirects (307 (string))]
-    expected: FAIL
-
-  [XMLHttpRequest: The send() method: POSTing to URL that redirects (307 (blob))]
-    expected: FAIL
-

tests/wpt/metadata/xhr/send-redirect-to-cors.htm.ini

@@ -1,35 +0,0 @@
-[send-redirect-to-cors.htm]
-  type: testharness
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (307 post with string)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (307 post with typed array)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (308 FOO with string and explicit Content-Type text/plain)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (308 FOO with string and explicit Content-Type multipart/form-data)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (308 FOO with string and explicit Content-Type safelisted)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (307 POST with string and explicit Content-Type safelisted)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (302 FOO with string and explicit Content-Type safelisted)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (301 POST with string and explicit Content-Type safelisted)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (303 FOO with string and explicit Content-Type safelisted)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (302 POST with string and explicit Content-Type)]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - Redirect to CORS-enabled resource (301 POST with string and explicit Content-Type)]
-    expected: FAIL
-