Commit e547241f authored by Hiroshige Hayashizaki's avatar Hiroshige Hayashizaki Committed by Commit Bot

MemoryCache: Do not reuse sync Resources for sync requests

For synchronous requests, Resource::WillFollowRedirect() is not
called and thus redirect chain is not set in Resources and
redirects are not reported to Resource.
Therefore, this breaks assumptions around redirects and
revalidations:
- The URLs of the original response and revalidation response
  can be different without WillFollowRedirect() calls,
  and this causes CHECK failure in Resource::RevalidationSucceeded().
- We forbid successful revalidation if either the old or new request
  is redirected, but this restriction relies on WillFollowRedirect()
  and thus is bypassed in sync cases.

This CL forbids reusing and revalidating synchronously-loaded
Resources for sync requests, to avoid these issues.

Bug: 618967
Change-Id: Ic8c668b751a69f594f3d08b7d63b9832b6bcc56e
Reviewed-on: https://chromium-review.googlesource.com/567679
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarTakeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486870}
parent 2b6e7ee2
......@@ -836,7 +836,18 @@ bool Resource::CanReuse(const FetchParameters& params) const {
//
// request_initiator_context is benign (indicates document vs. worker).
if (new_options.synchronous_policy != options_.synchronous_policy)
// Reuse only if both the existing Resource and the new request are
// asynchronous. Particularly,
// 1. Sync and async Resource/requests shouldn't be mixed (crbug.com/652172),
// 2. Sync existing Resources shouldn't be revalidated, and
// 3. Sync new requests shouldn't revalidate existing Resources.
//
// 2. and 3. are because SyncResourceHandler handles redirects without
// calling WillFollowRedirect, and causes response URL mismatch
// (crbug.com/618967) and bypassing redirect restriction around revalidation
// (crbug.com/613971 for 2. and crbug.com/614989 for 3.).
if (new_options.synchronous_policy == kRequestSynchronously ||
options_.synchronous_policy == kRequestSynchronously)
return false;
if (resource_request_.GetKeepalive() || new_request.GetKeepalive()) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment