New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 624004 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

CanonicalCookie::IsEquivalentForSecureCookieMatching does not work correctly.

Project Member Reported by mmenke@chromium.org, Jun 28 2016

Issue description

IsEquivalentForSecureCookieMatching uses CanonicalCookie.Source() to compare two cookies.  CanonicalCookie.Source() is *not* saved to disk, so any cookie loaded from disk will have an empty Source() GURL.  Any cookie set since the browser was started with have a non-empty Source() URL.

I believe the solution is just to use CanonicalCookie.Domain() instead of CanonicalCookie().Source().Host(), but someone more familiar with cookies should verify that.

I think we should get rid of the Source() field for just this reason, but that's another issue ( Issue 620770 , actually).
 

Comment 1 by mmenke@chromium.org, Jul 18 2016

Cc: jww@chromium.org
Owner: mmenke@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2830b0728b273d6fe15ecb28a1c27aa6cddd6c08

commit 2830b0728b273d6fe15ecb28a1c27aa6cddd6c08
Author: mmenke <mmenke@chromium.org>
Date: Wed Jul 20 16:02:50 2016

Fix CanonicalCookie::IsEquivalentForSecureCookieMatching

This method relies on the Source() of a cookie, which is not set
consistently. In particular, it's not set on cookies that have been
loaded from disk. This CL fixes the method, and removes storing the
source URL on cookies entirely to protect against future bugs.

Since the new implementation of IsEquivalentForSecureCookieMatching is
a little more expensive, also reorder code to call it a bit less often.

BUG= 620770 , 624004 

Review-Url: https://codereview.chromium.org/2158863003
Cr-Commit-Position: refs/heads/master@{#406567}

[modify] https://crrev.com/2830b0728b273d6fe15ecb28a1c27aa6cddd6c08/net/cookies/canonical_cookie.cc
[modify] https://crrev.com/2830b0728b273d6fe15ecb28a1c27aa6cddd6c08/net/cookies/canonical_cookie.h
[modify] https://crrev.com/2830b0728b273d6fe15ecb28a1c27aa6cddd6c08/net/cookies/cookie_monster.cc

Comment 3 by mmenke@chromium.org, Jul 20 2016

Status: Fixed (was: Assigned)

Sign in to add a comment