Remove unnecessary setAllowStoredCredentials() call in PingLoader::willFollowRedirect()

This doesn't have any effect on sendBeacon's credentials handling. This code was introduced in https://codereview.chromium.org/1016373002, but even since at this point, AllowStoredCredentials is passed to the BeaconLoader constructor. Replacing with DCHECK. R=mkwst@chromium.org BUG=636297 Review-Url: https://codereview.chromium.org/2336273003 Cr-Commit-Position: refs/heads/master@{#427048}

Remove unnecessary setAllowStoredCredentials() call in PingLoader::willFollowRedirect()
This doesn't have any effect on sendBeacon's credentials handling. This code was introduced in https://codereview.chromium.org/1016373002, but even since at this point, AllowStoredCredentials is passed to the BeaconLoader constructor. Replacing with DCHECK. R=mkwst@chromium.org BUG=636297 Review-Url: https://codereview.chromium.org/2336273003 Cr-Commit-Position: refs/heads/master@{#427048}
fbff05cf · tyoshino · Commit bot · a25e7a3d · fbff05cf · fbff05cf
Commit fbff05cf authored Oct 24, 2016 by tyoshino Committed by Commit bot Oct 24, 2016
3 changed files
--- a/third_party/WebKit/LayoutTests/http/tests/navigation/beacon-cross-origin-redirect-credentials.html
+++ b/third_party/WebKit/LayoutTests/http/tests/navigation/beacon-cross-origin-redirect-credentials.html
+<!DOCTYPE HTML>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/get-host-info.js?pipe=sub"></script>
+<script>
+if (window.testRunner)
+  testRunner.setBlockThirdPartyCookies(false);
+
+const host_info = get_host_info();
+
+document.cookie = "TestCookie=same";
+
+const set_cookie_promise = fetch(
+    host_info['HTTP_REMOTE_ORIGIN'] + '/security/resources/set-cookie.php?name=TestCookie&value=cross',
+    {mode: 'no-cors', credentials: 'include'});
+
+let count = 0;
+
+const test_name_prefix = 'cross-origin-redirect-credentials';
+
+function parse_check_beacon_response(body) {
+  const lines = body.split('\n');
+
+  const headers = {};
+
+  for (let line of lines) {
+    const elements = line.split(': ', 2);
+    const name = elements[0].toLowerCase();
+    const value = elements[1];
+    if (name in headers) {
+      headers[name].push(value);
+    } else {
+      headers[name] = [value];
+    }
+  }
+
+  return headers;
+}
+
+function run_test(url, status_code, expectations) {
+  const test_name = test_name_prefix + count;
+  ++count;
+
+  return set_cookie_promise.then(() => {
+    const destination_params = new URLSearchParams();
+    destination_params.append('name', test_name);
+    destination_params.append('dontclearcookies', undefined);
+
+    const params = new URLSearchParams();
+    params.append('url', url + '?' + destination_params.toString());
+    params.append('code', status_code);
+
+    navigator.sendBeacon('/resources/redirect.php?' + params.toString(), 'foobar');
+  }).then(() => {
+    return new Promise(resolve => {
+      setTimeout(resolve, 10);
+    });
+  }).then(() => {
+    return fetch('resources/check-beacon.php?name=' + test_name);
+  }).then(response => {
+    assert_equals(response.status, 200, 'check-beacon must be successful');
+
+    return response.text();
+  }).then(responseText => {
+    const headers = parse_check_beacon_response(responseText);
+
+    for (let name of Object.keys(expectations)) {
+      const expected_value = expectations[name];
+      if (expected_value === undefined) {
+        assert_false(name in headers, 'No ' + name + ' header expected');
+      } else {
+        assert_true(name in headers, name + ' header expected');
+        const actual_values = headers[name];
+        assert_equals(actual_values.length, 1, 'Just one ' + name + ' header expected');
+        assert_equals(actual_values[0],
+                      expected_value, 'Value of ' + name + ' header should be "' + expected_value + '"');
+      }
+    }
+  });
+}
+
+promise_test(() => {
+  return run_test(
+      host_info['HTTP_REMOTE_ORIGIN'] + '/navigation/resources/save-beacon.php',
+      301,
+      {
+        cookie: 'TestCookie=cross',
+        'request-method': 'GET'
+      });
+}, 'Status code 301');
+
+promise_test(() => {
+  return run_test(
+      host_info['HTTP_REMOTE_ORIGIN'] + '/navigation/resources/save-beacon.php',
+      302,
+      {
+        cookie: 'TestCookie=cross',
+        'request-method': 'GET'
+      });
+}, 'Status code 302');
+
+promise_test(() => {
+  return run_test(
+      host_info['HTTP_REMOTE_ORIGIN'] + '/navigation/resources/save-beacon.php',
+      303,
+      {
+        cookie: 'TestCookie=cross',
+        'request-method': 'GET'
+      });
+}, 'Status code 303');
+
+promise_test(() => {
+  return run_test(
+      host_info['HTTP_REMOTE_ORIGIN'] + '/navigation/resources/save-beacon.php',
+      307,
+      {
+        cookie: 'TestCookie=cross',
+        'request-method': 'POST'
+      });
+}, 'Status code 307');
+
+// TODO(tyoshino): Add a test for 308.
+</script>
--- a/third_party/WebKit/LayoutTests/http/tests/navigation/resources/save-beacon.php
+++ b/third_party/WebKit/LayoutTests/http/tests/navigation/resources/save-beacon.php
@@ -32,6 +32,9 @@ if (strpos($contentType, "application/") !== false) {
 fwrite($beaconFile, "Body: $postdata\n");
 fclose($beaconFile);
 rename($beaconFilename . ".tmp", $beaconFilename);
-foreach ($_COOKIE as $name => $value)
-    setcookie($name, "deleted", time() - 60, "/");
+
+if (!array_key_exists('dontclearcookies', $_GET)) {
+  foreach ($_COOKIE as $name => $value)
+      setcookie($name, "deleted", time() - 60, "/");
+}
 ?>
--- a/third_party/WebKit/Source/core/loader/PingLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/PingLoader.cpp
@@ -295,12 +295,11 @@ bool PingLoaderImpl::willFollowRedirect(
  if (!m_isBeacon)
    return true;

-  // TODO(tyoshino): Check if setAllowStoredCredentials() should be called also
-  // for non beacon cases.
-  passedNewRequest.setAllowStoredCredentials(true);
  if (m_corsMode == NotCORSEnabled)
    return true;

+  DCHECK(passedNewRequest.allowStoredCredentials());
+
  ResourceRequest& newRequest(passedNewRequest.toMutableResourceRequest());
  const ResourceResponse& redirectResponse(
      passedRedirectResponse.toResourceResponse());