Bug 62662 - inspector/cookie-parser.html is a flaky crash
Summary: inspector/cookie-parser.html is a flaky crash
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks: 79668
  Show dependency treegraph
 
Reported: 2011-06-14 13:47 PDT by WebKit Review Bot
Modified: 2014-12-12 14:09 PST (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description WebKit Review Bot 2011-06-14 13:47:15 PDT
[Chromium] inspector/cookie-parser.html is a flaky crash
Requested by dglazkov on #webkit.
Comment 2 Adam Barth 2011-06-14 14:45:06 PDT
Dare I ask why the inspector has a cookie parser?
Comment 3 Pavel Feldman 2011-06-14 22:49:00 PDT
(In reply to comment #2)
> Dare I ask why the inspector has a cookie parser?
Comment 4 Pavel Feldman 2011-06-14 23:00:10 PDT
> (In reply to comment #2)
> > Dare I ask why the inspector has a cookie parser?

Feature: Show Cookies and Set-Cookie per request

Cookie parsing is a part of the browser / network stack, i.e. platform-specific. None of the platforms expose APIs for parsing the cookies (Set-Cookie in particular). It often is encapsulated in the network stack. So the options are

a)
- expose parsing APIs on platforms (not always possible)
- extend WebKit API to include parsed structures (no super good reason for that)
- extend Web Inspector protocol to accompany raw headers with parsed structures (also not such a good reason to increase API surface)
b) 
- check whether cookie parsing is conservative enough and do a 100 line snippet with good enough quality that would do that for us

(b) it is.
Comment 5 Adam Barth 2011-06-14 23:28:02 PDT
Hopefully we use this parsing algorithm:

http://tools.ietf.org/html/rfc6265#section-5.2

:)
Comment 6 Pavel Feldman 2011-06-14 23:37:39 PDT
(In reply to comment #5)
> Hopefully we use this parsing algorithm:
> http://tools.ietf.org/html/rfc6265#section-5.2
> :)

I think caseq@ ported it from Chromium with its deviations from the RFCs accepted by Mozilla and IE.
Comment 7 Adam Barth 2011-06-14 23:49:09 PDT
> I think caseq@ ported it from Chromium with its deviations from the RFCs accepted by Mozilla and IE.

There shouldn't be any deviations from RFC 6265.  (That's the whole point of that spec.)  We're cleaning up any deviations that we discover to match that spec precisely.  If you know of any deviations, please let me know.
Comment 8 Pavel Feldman 2011-06-14 23:55:22 PDT
> There shouldn't be any deviations from RFC 6265.  (That's the whole point of that spec.)  We're cleaning up any deviations that we discover to match that spec precisely.  If you know of any deviations, please let me know.

Oh, I was not familiar with the effort. I implied deviations from the old 2109/2965. Things like http://codesearch.google.com/#OAMlx_jo-ck/src/net/base/cookie_monster.cc&exact_package=chromium&q=offer%5Csan%5Csoption%5Csfor%5Csquoted&l=1829. We'll probably need to sync our port once the cleanup is complete.
Comment 9 Adam Barth 2011-06-14 23:58:15 PDT
> Oh, I was not familiar with the effort. I implied deviations from the old 2109/2965.

Yes.  Those specs are not related to reality.

> Things like http://codesearch.google.com/#OAMlx_jo-ck/src/net/base/cookie_monster.cc&exact_package=chromium&q=offer%5Csan%5Csoption%5Csfor%5Csquoted&l=1829.

It's now clear how quoted strings should be handled.  I personally changed Firefox to match IE, Chrome, and RFC 6265.  Paul has promised to change CFNetwork to match as well.
Comment 10 Adam Barth 2011-06-15 00:05:22 PDT
I've posted http://codereview.chromium.org/7155016 to remove these out-of-date and confusing comments.
Comment 11 Pavel Feldman 2011-06-15 00:32:27 PDT
(In reply to comment #10)
> I've posted http://codereview.chromium.org/7155016 to remove these out-of-date and confusing comments.

Filed https://bugs.webkit.org/show_bug.cgi?id=62700 on inspector requiring its validation against the new rfc.
Comment 12 Csaba Osztrogonác 2011-07-11 03:31:44 PDT
It isn't Chromium specific bug, because this test asserts 
on Qt in debug mode with NRWT, but passes with ORWT.

error log with r90729:

ASSERTION FAILED: enabled || !supportsProfiling()
../../../Source/WebCore/bindings/js/JSDOMWindowBase.cpp(126) : virtual bool WebCore::JSDOMWindowBase::supportsRichSourceInfo() const
Comment 13 Balazs Kelemen 2011-10-19 06:50:52 PDT
(In reply to comment #12)
> It isn't Chromium specific bug, because this test asserts 
> on Qt in debug mode with NRWT, but passes with ORWT.
> 
> error log with r90729:
> 
> ASSERTION FAILED: enabled || !supportsProfiling()
> ../../../Source/WebCore/bindings/js/JSDOMWindowBase.cpp(126) : virtual bool WebCore::JSDOMWindowBase::supportsRichSourceInfo() const

After skip, the same assert failed with /inspector/cookie-resource-match-crash on Qt-Linux-x86_32-Debug: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Debug/r97849%20(19079)/inspector/cookie-resource-match-crash-log.txt
Comment 14 Csaba Osztrogonác 2011-10-19 06:54:31 PDT
It is the first inspector test ... maybe skipping it too solve the problem ...
Comment 15 Balazs Kelemen 2011-10-19 07:51:26 PDT
(In reply to comment #14)
> It is the first inspector test ... maybe skipping it too solve the problem ...

Let's try it. Skipped in http://trac.webkit.org/changeset/97862
Comment 16 Csaba Osztrogonác 2011-10-25 06:45:31 PDT
One more test skipped by http://trac.webkit.org/changeset/98338
Comment 17 Balazs Kelemen 2011-10-26 09:33:19 PDT
Trying a better approach to handle the bots by using test_expectations. Set the first one as CRASH+PASS: http://trac.webkit.org/changeset/98490.
Comment 18 Brian Burg 2014-12-12 14:09:04 PST
Closing as invalid, as this bug pertains to the old inspector UI and/or its tests.
Please file a new bug (https://www.webkit.org/new-inspector-bug) if the bug/feature/issue is still relevant to WebKit trunk.