Include Cookie tests to make sure "Set-Cookie: foo" and "Set-Cookie: =foo" work. |
|||||
Issue descriptionjkummerow@chromium.org is seeing a crash with the following backtrace on Chrome startup with a long-used profile: (gdb) bt #0 base::debug::BreakDebugger () at ../../base/debug/debugger_posix.cc:262 #1 0x00007f9214acff84 in logging::LogMessage::~LogMessage (this=0x7f91d2dc0d80) at ../../base/logging.cc:783 #2 0x00007f9213c3d2c7 in net::CanonicalCookie::Create (name="", value="undefined", domain="mobile.twitter.com", path="/", creation=..., expiration=..., last_access=..., secure=false, http_only=false, same_site=net::CookieSameSite::NO_RESTRICTION, priority=net::COOKIE_PRIORITY_MEDIUM) at ../../net/cookies/canonical_cookie.cc:238 #3 0x00007f920fa2bec4 in net::SQLitePersistentCookieStore::Backend::MakeCookiesFromSQLStatement (this=0x2866498628e0, cookies=0x7f91d2dc1330, statement=0x7f91d2dc13c8) at ../../net/extras/sqlite/sqlite_persistent_cookie_store.cc:818 Python Exception <type 'exceptions.IndexError'> list index out of range: #4 0x00007f920fa28148 in net::SQLitePersistentCookieStore::Backend::LoadCookiesForDomains (this=0x2866498628e0, domains=std::__debug::set with 8 elements) at ../../net/extras/sqlite/sqlite_persistent_cookie_store.cc:795 #5 0x00007f920fa27a53 in net::SQLitePersistentCookieStore::Backend::ChainLoadCookies(base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) (this=0x2866498628e0, loaded_callback=...) at ../../net/extras/sqlite/sqlite_persistent_cookie_store.cc:744 #6 0x00007f920cfb6f1f in base::internal::FunctorTraits<void (content::WebFileWriterImpl::WriterBridge::*)(base::Callback<void (base::File::Error), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&), void>::Invoke<scoped_refptr<content::WebFileWriterImpl::WriterBridge> const&, base::Callback<void (base::File::Error), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&>(void (content::WebFileWriterImpl::WriterBridge::*)(base::Callback<void (base::File::Error), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&), scoped_refptr<content::WebFileWriterImpl::WriterBridge> const&, base::Callback<void (base::File::Error), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) (method= (void (content::WebFileWriterImpl::WriterBridge::*)(content::WebFileWriterImpl::WriterBridge * const, const base::Callback<void (base::File::Error), base::internal::CopyMode::Copyable, base::internal::RepeatMode::Repeating> &)) 0x7f920fa278d0 <net::SQLitePersistentCookieStore::Backend::ChainLoadCookies(base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&)>, receiver_ptr=..., args=...) at ../../base/bind_internal.h:214 #7 0x00007f920cfb6e36 in base::internal::InvokeHelper<false, void>::MakeItSo<void (content::WebFileWriterImpl::WriterBridge::* const&)(base::Callback<void (base::File::Error), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&), scoped_refptr<content::WebFileWriterImpl::WriterBridge> const&, base::Callback<void (base::File::Error), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&>(void (content::WebFileWriterImpl::WriterBridge::* const&)(base::Callback<void (base::File::Error), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&), scoped_refptr<content::WebFileWriterImpl::WriterBridge> const&, base::Callback<void (base::File::Error), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) (functor= @0x28664a37eb48: (void (content::WebFileWriterImpl::WriterBridge::*)(content::WebFileWriterImpl::WriterBridge * const, const base::Callback<void (base::File::Error), base::internal::CopyMode::Copyable, base::internal::RepeatMode::Repeating> &)) 0x7f920fa278d0 <net::SQLitePersistentCookieStore::Backend::ChainLoadCookies(base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&)>, args=..., args=...) at ../../base/bind_internal.h:285 #8 0x00007f920fa32cc3 in base::internal::Invoker<base::internal::BindState<void (net::SQLitePersistentCookieStore::Backend::*)(base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&), scoped_refptr<net::SQLitePersistentCookieStore::Backend>, base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> >, void ()>::RunImpl<void (net::SQLitePersistentCookieStore::Backend::* const&)(base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&), std::tuple<scoped_refptr<net::SQLitePersistentCookieStore::Backend>, base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> > const&, 0ul, 1ul>(void (net::SQLitePersistentCookieStore::Backend::* const&)(base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&), std::tuple<scoped_refptr<net::SQLitePersistentCookieStore::Backend>, base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> > const&, base::IndexSequence<0ul, 1ul>) (functor= @0x28664a37eb48: (void (net::SQLitePersistentCookieStore::Backend::*)(net::SQLitePersistentCookieStore::Backend * const, const base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), base::internal::CopyMode::Copyable, base::internal::RepeatMode::Repeating> &)) 0x7f920fa278d0 <net::SQLitePersistentCookieStore::Backend::ChainLoadCookies(base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&)>, bound=empty std::tuple) at ../../base/bind_internal.h:361 #9 0x00007f920fa32bdc in base::internal::Invoker<base::internal::BindState<void (net::SQLitePersistentCookieStore::Backend::*)(base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&), scoped_refptr<net::SQLitePersistentCookieStore::Backend>, base::Callback<void (std::__debug::vector<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> >, std::allocator<std::unique_ptr<net::CanonicalCookie, std::default_delete<net::CanonicalCookie> > > >), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> >, void ()>::Run(base::internal::BindStateBase*) (base=0x28664a37eb20) at ../../base/bind_internal.h:339 #10 0x00007f9214a18a5e in base::Callback<void (), (base::internal::CopyMode)0, (base::internal::RepeatMode)0>::Run() && (this=0x286649f62b98) at ../../base/callback.h:91 #11 0x00007f9214a62ece in base::debug::TaskAnnotator::RunTask (this=0x7f91d2dc1de0, queue_function=0x7f9214dadf40 <base::internal::(anonymous namespace)::kQueueFunctionName> "base::PostTask", pending_task=0x286649f62b80) at ../../base/debug/task_annotator.cc:59 #12 0x00007f9214c24238 in base::internal::TaskTracker::PerformRunTask (this=0x28664791c118, task=std::unique_ptr<base::internal::Task> containing 0x286649f62b80) at ../../base/task_scheduler/task_tracker.cc:331 #13 0x00007f9214c26455 in base::internal::TaskTrackerPosix::PerformRunTask (this=0x28664791c118, task=std::unique_ptr<base::internal::Task> containing 0x0) at ../../base/task_scheduler/task_tracker_posix.cc:21 #14 0x00007f9214c236ca in base::internal::TaskTracker::RunTask (this=0x28664791c118, task=std::unique_ptr<base::internal::Task> containing 0x0, sequence_token=...) at ../../base/task_scheduler/task_tracker.cc:293 #15 0x00007f9214c10c3b in base::internal::SchedulerWorker::Thread::ThreadMain (this=0x2866498a39e0) at ../../base/task_scheduler/scheduler_worker.cc:79 #16 0x00007f9214c3200a in base::(anonymous namespace)::ThreadFunc (params=0x2866492994d0) at ../../base/threading/platform_thread_posix.cc:71 #17 0x00007f92150c9184 in start_thread (arg=0x7f91d2dc3700) at pthread_create.c:312 #18 0x00007f91f9557bed in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111 This is happening because a cookie being retrieved from the cookie persistent store has no name, and I added a DCHECK for cookies with no name as part of my recent changes. Jakob, what's the revision/commit that this build was built at? I've been doing a lot of churn in the code base, and I suspect the current code base won't crash like this (though I think there's still a bug in it). But I'd still like to have pretty clear repro instructions; hopefully the result of this bug will be a test that confirms this doesn't keep happening.
,
May 18 2017
The backtrace is from c7131815b9cd8ff9257fe8f274f96e5b400b437e / r472044. With 43102f798945c3fb8179e9841046d4015ebe9127 / r472747 (ToT right now), it doesn't repro any more.
,
May 18 2017
This DCHECK crash was caused by http://codereview.chromium.org/2861063003, which added DCHECKs making sure that the name and path passed to the unvalidated CanonicalCookie::Create() method were non-null. (The SQL DB on your machine apparently has a null name in one of its cookies.) This crash will no longer occur after http://codereview.chromium.org/2874843002, since that Create was shifted over to a constructor, which didn't have the DCHECKs in it. While I'd like to fix that loss of DCHECK, that does mean that this bug doesn't need to block 722827.
,
May 18 2017
Matt, sharing context with you for a small piece of a CL I'll be asking you to review reasonably soon. This bug occurred when I put some DCHECKs when I nuked the dangerous Create method (to make the unvalidated constructor a bit safer) but then I accidentally removed them when I switched over to using the constructor instead of the unvalidated Create method. I could put them back in (and clean up this call site), but the approach I decided instead to take is to let the constructor construct arbitrary CanonicalCookies without any DCHECKs and have the various locations that care about validity of those cookies call an IsCanonical() method on those cookies before accepting them. I could DCHECK(IsCanonical()) at creation time for CanonicalCookies, but that removes from the callers the easiest way to check if they've got a valid cookie.
,
May 25 2017
Randy: Branch is today. Do we need a fix for this in M60, or is this harmless? I'm not exactly following what the problem is.
,
May 25 2017
Harmless; it is not currently occurring in trunk. The underlying issue (not sanitizing cookies on read in from disk) is handled in my current CL.
,
Jun 6 2017
So it turns out that this was an error on my part and I should never have landed that DCHECK. See https://codereview.chromium.org/2898953008/diff/1/net/cookies/canonical_cookie.cc#newcode405. This crash is not currently happening in trunk, so I'm going to officially switch this bug over to adding tests to make sure this is caught if someone makes the same mistake (of paying attention to the spec) that I did.
,
Jun 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1e64af8eed07578f01b933d654180659c0fe962c commit 1e64af8eed07578f01b933d654180659c0fe962c Author: rdsmith <rdsmith@chromium.org> Date: Sat Jun 17 00:25:28 2017 Implement and test CanonicalCookie::IsCanonical() This is done for data validation including but not limited to the cookies servicification effort. BUG= 721395 , 723734 R=mmenke@chromium.org Review-Url: https://codereview.chromium.org/2898953008 Cr-Commit-Position: refs/heads/master@{#480238} [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/android_webview/browser/net/aw_cookie_store_wrapper_unittest.cc [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/ios/net/cookies/cookie_store_ios_persistent_unittest.mm [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/ios/net/cookies/cookie_store_ios_unittest.mm [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/net/cookies/canonical_cookie.cc [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/net/cookies/canonical_cookie.h [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/net/cookies/canonical_cookie_unittest.cc [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/net/cookies/cookie_monster_unittest.cc [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/net/cookies/cookie_store_unittest.h [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/net/cookies/parsed_cookie.cc [modify] https://crrev.com/1e64af8eed07578f01b933d654180659c0fe962c/net/cookies/parsed_cookie.h
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3df4c7446b9f3421636917277428f71a306a857f commit 3df4c7446b9f3421636917277428f71a306a857f Author: rdsmith <rdsmith@chromium.org> Date: Wed Jun 21 00:31:43 2017 Fix loading bad cookies from disk. BUG= 723734 R=mmenke@chromium.org Review-Url: https://codereview.chromium.org/2909533002 Cr-Commit-Position: refs/heads/master@{#481043} [modify] https://crrev.com/3df4c7446b9f3421636917277428f71a306a857f/net/extras/sqlite/sqlite_persistent_cookie_store.cc [modify] https://crrev.com/3df4c7446b9f3421636917277428f71a306a857f/net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc
,
Jun 21 2017
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a6ce44474ee162592d04a31cf0b243c31a53f57d commit a6ce44474ee162592d04a31cf0b243c31a53f57d Author: rdsmith <rdsmith@chromium.org> Date: Wed Jun 21 17:11:05 2017 Add a SetCanonicalCookie method for CookieMonster. This includes some refactoring of creation time defaulting, as well as a histogram revision bump because the information about whether a cookie is being set based on a null URL is no longer available at the point of histogram creation. BUG= 721395 , 723734 R=mmenke@chromium.org Review-Url: https://codereview.chromium.org/2882063002 Cr-Commit-Position: refs/heads/master@{#481227} [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/android_webview/browser/net/aw_cookie_store_wrapper.cc [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/android_webview/browser/net/aw_cookie_store_wrapper.h [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/chrome/browser/extensions/api/cookies/cookies_unittest.cc [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/headless/public/util/testing/generic_url_request_mocks.cc [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/headless/public/util/testing/generic_url_request_mocks.h [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/ios/net/cookies/cookie_store_ios.h [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/ios/net/cookies/cookie_store_ios.mm [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/ios/net/cookies/cookie_store_ios_persistent.h [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/ios/net/cookies/cookie_store_ios_persistent.mm [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/canonical_cookie.cc [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/canonical_cookie.h [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/canonical_cookie_unittest.cc [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/cookie_monster.cc [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/cookie_monster.h [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/cookie_monster_unittest.cc [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/cookie_store.h [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/cookie_store_test_helpers.cc [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/cookie_store_test_helpers.h [modify] https://crrev.com/a6ce44474ee162592d04a31cf0b243c31a53f57d/net/cookies/cookie_store_unittest.h |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by gab@chromium.org
, May 17 2017