This repository has been archived by the owner. It is now read-only.

Separate CookieJar objects for WebPages #11417

Open
jrollinson opened this Issue Jun 18, 2013 · 15 comments

Comments

Projects
None yet
8 participants
@jrollinson

jrollinson commented Jun 18, 2013

Currently, there exists a single CookieJar object for all web pages. This is in line with the usual behaviour of web pages, but causes difficulty when treating pages as separate entities.

To fix: CookieJar should not be a singleton, and objects that use a CookieJar should have one passed in on construction.

@jrollinson

This comment has been minimized.

Show comment
Hide comment
@jrollinson

jrollinson commented Jun 18, 2013

Relevant Ghostdriver issues:
detro/ghostdriver#90
detro/ghostdriver#170

@ariya

This comment has been minimized.

Show comment
Hide comment
@ariya

ariya Jun 19, 2013

Owner

Interesting. Let me come up with a hypothetical API and code:

var myJar = new CookieJar();
page.cookieJar = myJar;
myJar.save('/secret/path/to/my/locker/cookie.dat');

Does that make sense? Obviously, there can be a default cookie jar for a new page instance.

Owner

ariya commented Jun 19, 2013

Interesting. Let me come up with a hypothetical API and code:

var myJar = new CookieJar();
page.cookieJar = myJar;
myJar.save('/secret/path/to/my/locker/cookie.dat');

Does that make sense? Obviously, there can be a default cookie jar for a new page instance.

@JamesMGreene

This comment has been minimized.

Show comment
Hide comment
@JamesMGreene

JamesMGreene Jun 19, 2013

Collaborator

What about for loading an existing CookieJar? Specify a path in the constructor, maybe?

Collaborator

JamesMGreene commented Jun 19, 2013

What about for loading an existing CookieJar? Specify a path in the constructor, maybe?

@jrollinson

This comment has been minimized.

Show comment
Hide comment
@jrollinson

jrollinson Jun 19, 2013

@ariya Are you suggesting a new CookieJar module like this?

var cookieJarCreator = require('cookieJar');
var myJar = cookieJarCreator.create();

jrollinson commented Jun 19, 2013

@ariya Are you suggesting a new CookieJar module like this?

var cookieJarCreator = require('cookieJar');
var myJar = cookieJarCreator.create();
@jrollinson

This comment has been minimized.

Show comment
Hide comment
@jrollinson

jrollinson Jun 19, 2013

I think an optional path in the constructor would be a great way for loading existing cookie jars.

jrollinson commented Jun 19, 2013

I think an optional path in the constructor would be a great way for loading existing cookie jars.

@ariya

This comment has been minimized.

Show comment
Hide comment
@ariya

ariya Jun 19, 2013

Owner

Whether it's in its own module, conveniently constructed, etc are small details. What we need to come up with is the answer to the following question "if such an API is provided, how would your code look like".

Owner

ariya commented Jun 19, 2013

Whether it's in its own module, conveniently constructed, etc are small details. What we need to come up with is the answer to the following question "if such an API is provided, how would your code look like".

@jrollinson

This comment has been minimized.

Show comment
Hide comment
@jrollinson

jrollinson Jun 19, 2013

Ok. I think the code would look like this:

var myJar = new CookieJar();
page1.cookieJar = myJar;
page2.cookieJar = myJar;
myJar.addCookie(cookie);

... do whatever ...

page1.close();
page2.close();
myJar.save(path);
myJar.close();

A new page would have the default CookieJar and then you can set the page's CookieJar to be whatever you want.
Making the user explicitly close the CookieJar would save a bit of a nightmare.

Also, I think that the CookieJar should have the same API that both the phantom and webpage objects have with cookies.
In other words it should have the following functions and properties:

addCookie(Cookie);
deleteCookie(cookieName);
cookies;
clearCookies();
save(path);
close();

We should also add a CookieJar getter on the phantom object and both a getter and a setter for the CookieJar in webpages.

jrollinson commented Jun 19, 2013

Ok. I think the code would look like this:

var myJar = new CookieJar();
page1.cookieJar = myJar;
page2.cookieJar = myJar;
myJar.addCookie(cookie);

... do whatever ...

page1.close();
page2.close();
myJar.save(path);
myJar.close();

A new page would have the default CookieJar and then you can set the page's CookieJar to be whatever you want.
Making the user explicitly close the CookieJar would save a bit of a nightmare.

Also, I think that the CookieJar should have the same API that both the phantom and webpage objects have with cookies.
In other words it should have the following functions and properties:

addCookie(Cookie);
deleteCookie(cookieName);
cookies;
clearCookies();
save(path);
close();

We should also add a CookieJar getter on the phantom object and both a getter and a setter for the CookieJar in webpages.

@JoshGalvin

This comment has been minimized.

Show comment
Hide comment
@JoshGalvin

JoshGalvin Jun 21, 2013

Running into this problem with trying to run tests in parallel in CasperJS. Having the ability to set CookieJars per instance of WebPage would be extremely helpful.

JoshGalvin commented Jun 21, 2013

Running into this problem with trying to run tests in parallel in CasperJS. Having the ability to set CookieJars per instance of WebPage would be extremely helpful.

@jrollinson

This comment has been minimized.

Show comment
Hide comment
@jrollinson

jrollinson Jun 28, 2013

I would like to be responsible for this issue. What do you think about the interface I described above?

jrollinson commented Jun 28, 2013

I would like to be responsible for this issue. What do you think about the interface I described above?

@brokenthumbs

This comment has been minimized.

Show comment
Hide comment
@brokenthumbs

brokenthumbs Sep 12, 2013

Any chance that this update could be pulled in to PhantomJS? Would love to use PhantomJS with GhostDriver and run sessions in parallel, and this would help eliminate any shared cookie issues, to mitigate issues concerning logging in as different users and such.

brokenthumbs commented Sep 12, 2013

Any chance that this update could be pulled in to PhantomJS? Would love to use PhantomJS with GhostDriver and run sessions in parallel, and this would help eliminate any shared cookie issues, to mitigate issues concerning logging in as different users and such.

@artkoshelev

This comment has been minimized.

Show comment
Hide comment
@artkoshelev

artkoshelev Dec 3, 2013

waiting for this issues as well, it's a show-stopper for using PhantomJS with GhostDriver in our web-tests

artkoshelev commented Dec 3, 2013

waiting for this issues as well, it's a show-stopper for using PhantomJS with GhostDriver in our web-tests

jrollinson added a commit to jrollinson/phantomjs that referenced this issue Jan 6, 2014

Re-integrates the qDebug messages for cookie jar
Adds qDebug messages to CookieJar constructor.

ariya#11417

jrollinson added a commit to jrollinson/phantomjs that referenced this issue Jan 6, 2014

Renames m_cookieJarSingleton to m_defaultCookieJar
m_cookeJarSingleton has changed from being the only cookie jar to being the
default cookie jar.
This warrants a name change.

ariya#11417

jrollinson added a commit to jrollinson/phantomjs that referenced this issue Jan 6, 2014

jrollinson added a commit to jrollinson/phantomjs that referenced this issue Jan 6, 2014

Makes CustomPage constructor default parent to NULL.
Changes order of constructor parameters so that the parent is last and can be
defaulted to NULL.

ariya#11417

jrollinson added a commit to jrollinson/phantomjs that referenced this issue Jan 6, 2014

Makes cookieToMap in CookieJar a public slot.
Previously cookieToMap was a public function.
Since cookiesToMap is a public slot, this change makes the cookie jar
interface more symmetric.

ariya#11417

jrollinson added a commit to jrollinson/phantomjs that referenced this issue Jan 7, 2014

Adds support for multiple Cookie Jars.
Previously, there was a single global cookie jar shared between all web pages.
Now, one can have separate cookie jars for different web pages.

Makes CookieJar a normal class, not a singleton.
Moves many public CookieJar methods to public slots.
Adds default cookie jar to Phantom.
Adds the CookieJar module that provides access to cookie jars in javascript.
Adds cookie jar module tests.

Usage:
var jar = require('cookiejar').create();
var webpage = require('webpage').create();
webpage.cookieJar = jar;
...
webpage.close();
jar.close();

JS API changes:
Webpage:
    var jar = page.cookieJar; -- assigns 'jar' the given webpage's cookie jar.
    page.cookiejar = jar; -- sets 'jar' as the given webpage's cookie jar.
CookieJar:
    var jar = require('cookiejar').create(path)
        creates a cookie jar with persistent storage at the given file path
        (path not mandatory).
    var cookies = jar.cookies; -- assign's 'jar' the list of cookies in the
        cookie jar.
    jar.cookies = [c1, c2]; -- sets the cookie jar's cookies as the ones in the
        list.
    jar.addCookie(cookie) -- adds cookie 'cookie' to the cookie jar.

fixes issue #11417

jrollinson added a commit to jrollinson/phantomjs that referenced this issue Jan 10, 2014

Adds support for multiple Cookie Jars.
Previously, there was a single global cookie jar shared between all web pages.
Now, one can have separate cookie jars for different web pages.

Makes CookieJar a normal class, not a singleton.
Moves many public CookieJar methods to public slots.
Adds default cookie jar to Phantom.
Adds the CookieJar module that provides access to cookie jars in javascript.
Adds cookie jar module tests.

Usage:
var jar = require('cookiejar').create();
var webpage = require('webpage').create();
webpage.cookieJar = jar;
...
webpage.close();
jar.close();

JS API changes:
Webpage:
    var jar = page.cookieJar; -- assigns 'jar' the given webpage's cookie jar.
    page.cookiejar = jar; -- sets 'jar' as the given webpage's cookie jar.
CookieJar:
    var jar = require('cookiejar').create(path)
        creates a cookie jar with persistent storage at the given file path
        (path not mandatory).
    var cookies = jar.cookies; -- assign's 'jar' the list of cookies in the
        cookie jar.
    jar.cookies = [c1, c2]; -- sets the cookie jar's cookies as the ones in the
        list.
    jar.addCookie(cookie) -- adds cookie 'cookie' to the cookie jar.

fixes issue #11417

jrollinson added a commit to jrollinson/phantomjs that referenced this issue Jan 10, 2014

Adds support for multiple Cookie Jars.
Previously, there was a single global cookie jar shared between all web pages.
Now, one can have separate cookie jars for different web pages.

Makes CookieJar a normal class, not a singleton.
Moves many public CookieJar methods to public slots.
Adds default cookie jar to Phantom.
Adds the CookieJar module that provides access to cookie jars in javascript.
Adds cookie jar module tests.

Usage:
var jar = require('cookiejar').create();
var webpage = require('webpage').create();
webpage.cookieJar = jar;
...
webpage.close();
jar.close();

JS API changes:
Webpage:
    var jar = page.cookieJar; -- assigns 'jar' the given webpage's cookie jar.
    page.cookiejar = jar; -- sets 'jar' as the given webpage's cookie jar.
CookieJar:
    var jar = require('cookiejar').create(path)
        creates a cookie jar with persistent storage at the given file path
        (path not mandatory).
    var cookies = jar.cookies; -- assign's 'jar' the list of cookies in the
        cookie jar.
    jar.cookies = [c1, c2]; -- sets the cookie jar's cookies as the ones in the
        list.
    jar.addCookie(cookie) -- adds cookie 'cookie' to the cookie jar.

fixes issue #11417

ariya added a commit that referenced this issue Jan 11, 2014

Adds support for multiple Cookie Jars.
Previously, there was a single global cookie jar shared between all web pages.
Now, one can have separate cookie jars for different web pages.

Makes CookieJar a normal class, not a singleton.
Moves many public CookieJar methods to public slots.
Adds default cookie jar to Phantom.
Adds the CookieJar module that provides access to cookie jars in javascript.
Adds cookie jar module tests.

Usage:
var jar = require('cookiejar').create();
var webpage = require('webpage').create();
webpage.cookieJar = jar;
...
webpage.close();
jar.close();

JS API changes:
Webpage:
    var jar = page.cookieJar; -- assigns 'jar' the given webpage's cookie jar.
    page.cookiejar = jar; -- sets 'jar' as the given webpage's cookie jar.
CookieJar:
    var jar = require('cookiejar').create(path)
        creates a cookie jar with persistent storage at the given file path
        (path not mandatory).
    var cookies = jar.cookies; -- assign's 'jar' the list of cookies in the
        cookie jar.
    jar.cookies = [c1, c2]; -- sets the cookie jar's cookies as the ones in the
        list.
    jar.addCookie(cookie) -- adds cookie 'cookie' to the cookie jar.

#11417
@yelizariev

This comment has been minimized.

Show comment
Hide comment
@yelizariev

yelizariev Sep 2, 2016

This issue is closed, I guess

yelizariev commented Sep 2, 2016

This issue is closed, I guess

@Vitallium

This comment has been minimized.

Show comment
Hide comment
@Vitallium

Vitallium Sep 2, 2016

Collaborator

It's not closed and this problem is in out TODO list. I believe this is a crucial feature and it should be implemented.

Collaborator

Vitallium commented Sep 2, 2016

It's not closed and this problem is in out TODO list. I believe this is a crucial feature and it should be implemented.

@yelizariev

This comment has been minimized.

Show comment
Hide comment
@yelizariev

yelizariev Sep 2, 2016

@Vitallium have you tried 2.0.0+ version? 244cf25

yelizariev commented Sep 2, 2016

@Vitallium have you tried 2.0.0+ version? 244cf25

@Vitallium

This comment has been minimized.

Show comment
Hide comment
@Vitallium

Vitallium Sep 2, 2016

Collaborator

The problem is that in the current state this feature requires some work to do. For example. it can't handle when the user closes a web page. We need to improve it and fix some nasty bugs.

Collaborator

Vitallium commented Sep 2, 2016

The problem is that in the current state this feature requires some work to do. For example. it can't handle when the user closes a web page. We need to improve it and fix some nasty bugs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.