Bug 32160 - Web Inspector: Allow retrieval of all transmitted cookies
Summary: Web Inspector: Allow retrieval of all transmitted cookies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 32223
  Show dependency treegraph
 
Reported: 2009-12-04 08:22 PST by Alexander Pavlov (apavlov)
Modified: 2009-12-08 05:21 PST (History)
9 users (show)

See Also:


Attachments
patch (13.10 KB, patch)
2009-12-04 09:02 PST, Alexander Pavlov (apavlov)
pfeldman: review-
Details | Formatted Diff | Diff
patch (fixed) (18.52 KB, patch)
2009-12-07 08:02 PST, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
patch (add comment + use match group) (18.58 KB, patch)
2009-12-07 10:51 PST, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
patch (test cleanup) (18.58 KB, patch)
2009-12-08 05:02 PST, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pavlov (apavlov) 2009-12-04 08:22:15 PST
Currently there is no way to retrieve all cookies in Web Inspector, which is required for audits.
Comment 1 Alexander Pavlov (apavlov) 2009-12-04 09:02:34 PST
Created attachment 44318 [details]
patch

The approach has been discussed with pfeldman@chromium.org
Comment 2 WebKit Review Bot 2009-12-04 09:07:09 PST
style-queue ran check-webkit-style on attachment 44318 [details] without any errors.
Comment 3 Pavel Feldman 2009-12-04 12:16:51 PST
Comment on attachment 44318 [details]
patch

> +        for (var c = 0; c < allCookies.length; ++c) {

c is a strange index name.

> +            for (var id in WebInspector.resources) {
> +                var resource = WebInspector.resources[id];
> +                var match = resource.documentURL.match(WebInspector.URLRegExp);

You don't need to do matching numCookeis x numResources times. It is easier to go though resources and store their domains into a set first (numResources steps), then you can get all cookies for given frame within numCookies x numDomains steps.

>  
> +WebInspector.Cookies.cookieMatchesResourceURL = function(cookie, resourceURL)
> +{

I don't see this method used.

> +
> +WebInspector.Cookies.cookieDomainMatchesResourceDomain = function(cookieDomain, resourceDomain)
> +{
> +    if (cookieDomain.charAt(0) !== '.')
> +        return resourceDomain === cookieDomain;
> +    return !!resourceDomain.match(new RegExp("^([^\\.]+\\.)?" + cookieDomain.substring(1).escapeForRegExp() + "$"), "i");

A test for this would be nice. You need to do a inspector test that would match a bunch of URLs against given domain on the frontend side.
Comment 4 Alexander Pavlov (apavlov) 2009-12-07 08:01:13 PST
(In reply to comment #3)
> (From update of attachment 44318 [details])
> > +        for (var c = 0; c < allCookies.length; ++c) {
> 
> c is a strange index name.

i now

> > +            for (var id in WebInspector.resources) {
> > +                var resource = WebInspector.resources[id];
> > +                var match = resource.documentURL.match(WebInspector.URLRegExp);
> 
> You don't need to do matching numCookeis x numResources times. It is easier to
> go though resources and store their domains into a set first (numResources
> steps), then you can get all cookies for given frame within numCookies x
> numDomains steps.

We should store full URLs rather than domains (since the cookies are matched using the protocol, hostname, port, and path.) I have changed the impl, so we can gain some benefit when there are several cookie domains in the page.

> >  
> > +WebInspector.Cookies.cookieMatchesResourceURL = function(cookie, resourceURL)
> > +{
> 
> I don't see this method used.

Thanks for the catch. For some reason I was invoking domain matching rather than full matching.

> > +
> > +WebInspector.Cookies.cookieDomainMatchesResourceDomain = function(cookieDomain, resourceDomain)
> > +{
> > +    if (cookieDomain.charAt(0) !== '.')
> > +        return resourceDomain === cookieDomain;
> > +    return !!resourceDomain.match(new RegExp("^([^\\.]+\\.)?" + cookieDomain.substring(1).escapeForRegExp() + "$"), "i");
> 
> A test for this would be nice. You need to do a inspector test that would match
> a bunch of URLs against given domain on the frontend side.

Done.
Comment 5 Alexander Pavlov (apavlov) 2009-12-07 08:02:15 PST
Created attachment 44417 [details]
patch (fixed)
Comment 6 WebKit Review Bot 2009-12-07 08:05:00 PST
style-queue ran check-webkit-style on attachment 44417 [details] without any errors.
Comment 7 Alexander Pavlov (apavlov) 2009-12-07 10:51:26 PST
Created attachment 44421 [details]
patch (add comment + use match group)
Comment 8 WebKit Review Bot 2009-12-07 10:52:44 PST
style-queue ran check-webkit-style on attachment 44421 [details] without any errors.
Comment 9 Alexander Pavlov (apavlov) 2009-12-08 05:02:46 PST
Created attachment 44467 [details]
patch (test cleanup)
Comment 10 WebKit Review Bot 2009-12-08 05:05:04 PST
style-queue ran check-webkit-style on attachment 44467 [details] without any errors.
Comment 11 WebKit Commit Bot 2009-12-08 05:21:11 PST
Comment on attachment 44467 [details]
patch (test cleanup)

Clearing flags on attachment: 44467

Committed r51846: <http://trac.webkit.org/changeset/51846>
Comment 12 WebKit Commit Bot 2009-12-08 05:21:17 PST
All reviewed patches have been landed.  Closing bug.