Update HTTP-redirect fetch

servo · Aug 18, 2017 · d6c197b · d6c197b
1 parent 0ff47c4
commit d6c197b
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 30 deletions.
diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs
@@ -650,29 +650,20 @@ pub fn http_redirect_fetch(request: &mut Request,
     // Step 1
     assert!(response.return_internal);
 
-    // Step 2
-    if !response.actual_response().headers.has::<Location>() {
-        return response;
-    }
-
-    // Step 3
-    let location = match response.actual_response().headers.get::<Location>() {
-        Some(&Location(ref location)) => location.clone(),
-        _ => return Response::network_error(NetworkError::Internal("Location header parsing failure".into()))
-    };
-    let response_url = response.actual_response().url().unwrap();
-    let location_url = response_url.join(&*location);
+    let location_url = response.actual_response().location_url.clone();
     let location_url = match location_url {
-        Ok(url) => url,
-        _ => return Response::network_error(NetworkError::Internal("Location URL parsing failure".into()))
+        // Step 2
+        None => return response,
+        // Step 3
+        Some(Err(err)) =>
+            return Response::network_error(
+                NetworkError::Internal("Location URL parse failure: ".to_owned() + &err)),
+        // Step 4
+        Some(Ok(ref url)) if !matches!(url.scheme(), "http" | "https") =>
+            return Response::network_error(NetworkError::Internal("Location URL not an HTTP(S) scheme".into())),
+        Some(Ok(url)) => url,
     };
 
-    // Step 4
-    match location_url.scheme() {
-        "http" | "https" => { },
-        _ => return Response::network_error(NetworkError::Internal("Not an HTTP(S) Scheme".into()))
-    }
-
     // Step 5
     if request.redirect_count >= 20 {
         return Response::network_error(NetworkError::Internal("Too many redirects".into()));
@@ -682,7 +673,8 @@ pub fn http_redirect_fetch(request: &mut Request,
     request.redirect_count += 1;
 
     // Step 7
-    let same_origin = location_url.origin()== request.current_url().origin();
+    // FIXME: Correctly use request's origin
+    let same_origin = location_url.origin() == request.current_url().origin();
     let has_credentials = has_credentials(&location_url);
 
     if request.mode == RequestMode::CorsMode && !same_origin && has_credentials {
@@ -695,28 +687,38 @@ pub fn http_redirect_fetch(request: &mut Request,
     }
 
     // Step 9
+    if response.actual_response().status.map_or(true, |s| s != StatusCode::SeeOther) &&
+       request.body.as_ref().map_or(false, |b| b.is_empty()) {
+        return Response::network_error(NetworkError::Internal("Request body is not done".into()));
+    }
+
+    // Step 10
     if cors_flag && !same_origin {
         request.origin = Origin::Origin(ImmutableOrigin::new_opaque());
     }
 
-    // Step 10
-    let status_code = response.actual_response().status.unwrap();
-    if ((status_code == StatusCode::MovedPermanently || status_code == StatusCode::Found) &&
-        request.method == Method::Post) ||
-        status_code == StatusCode::SeeOther {
+    // Step 11
+    if response.actual_response().status.map_or(false, |code|
+        ((code == StatusCode::MovedPermanently || code == StatusCode::Found) && request.method == Method::Post) ||
+        code == StatusCode::SeeOther) {
         request.method = Method::Get;
         request.body = None;
     }
 
-    // Step 11
+    // Step 12
+    if let Some(_) = request.body {
+        // TODO: extract request's body's source
+    }
+
+    // Step 13
     request.url_list.push(location_url);
 
-    // Step 12
+    // Step 14
     // TODO implement referrer policy
 
+    // Step 15
     let recursive_flag = request.redirect_mode != RedirectMode::Manual;
 
-    // Step 13
     main_fetch(request, cache, cors_flag, recursive_flag, target, done_chan, context)
 }
 

diff --git a/components/net_traits/request.rs b/components/net_traits/request.rs
@@ -220,7 +220,7 @@ pub struct Request {
     // TODO: target browsing context
     /// https://fetch.spec.whatwg.org/#request-keepalive-flag
     pub keep_alive: bool,
-    // https://fetch.spec.whatwg.org/#request-service-workers-mode
+    /// https://fetch.spec.whatwg.org/#request-service-workers-mode
     pub service_workers_mode: ServiceWorkersMode,
     /// https://fetch.spec.whatwg.org/#concept-request-initiator
     pub initiator: Initiator,