CookieManager.add doesn't accept cookie format supplied by CookieManager.all #295

Closed
meeotch opened this Issue Feb 6, 2014 · 4 comments

Comments

Projects
None yet
3 participants
@meeotch

meeotch commented Feb 6, 2014

Browser.cookies.add() doesn't seem to like the cookies returned by Browser.cookies.all(). To reproduce:

>> b.visit("http://google.com")
>> acookie = b.cookies.all()[0]
>> acookie
{u'domain': u'.google.com', u'name': u'PREF', u'value': u'ID=d1c9bd113ed75b77:U=edf4f5b25a8ab183:FF=0:TM=1391716688:LM=1391716690:S=VfO-BVRoKQvZKcBX', u'expiry': 1454788681, u'path': u'/', u'secure': False}
>> b.cookies.delete()
>> b.cookies.add(acookie)
>> b.cookies.all()
[{u'domain': u'www.google.com', u'name': u'domain', u'value': u'.google.com', u'expiry': 1896639921, u'path': u'', u'secure': False},
 {u'domain': u'www.google.com', u'name': u'name', u'value': u'PREF', u'expiry': 1896639921, u'path': u'', u'secure': False},
 {u'domain': u'www.google.com', u'name': u'value', u'value': u'ID=7c4c2e6d39468d53:U=51495c50c0c4abd0:FF=0:TM=1391718289:LM=1391718290:S=-ccEkmggULDSHK5w', u'expiry': 1896639921, u'path': u'', u'secure': False},
 {u'domain': u'www.google.com', u'name': u'expiry', u'value': u'1454790282', u'expiry': 1896639921, u'path': u'', u'secure': False},
 {u'domain': u'www.google.com', u'name': u'path', u'value': u'/', u'expiry': 1896639921, u'path': u'', u'secure': False},
 {u'domain': u'www.google.com', u'name': u'secure', u'value': u'false', u'expiry': 1896639921, u'path': u'', u'secure': False}]

Note that it sets several cookies, one for each key of the original cookie. Using instead

>> b.cookies.driver.add_cookie(acookie)

works as expected.

Seems like the behavior of the two functions should be consistent, or at least the docs should mention the discrepancy. As a new user (to both splinter and selenium), it took me a while to figure this out. I'd think that the simple case of saving a batch (no pun intended) of cookies and re-loading them into the browser should be more intuitive.

@douglascamata

This comment has been minimized.

Show comment Hide comment
@douglascamata

douglascamata Feb 7, 2014

Member

That's how Selenium cookie handler "works". In this code, for example:

>>> b = Browser()
>>> b.visit('http://google.com')
>>> b.cookies.all()
...
>>> b.cookies.all()[0]
{u'domain': u'.google.com.br',
 u'expiry': 1407542980,
 u'name': u'NID',
 u'path': u'/',
 u'secure': False,
 u'value': u'67=HPC_MzkBCnFlbZAKMFdnGhT4ZefznuCkvRabmGANtkyMAdEcGjrORYM2F_UoW9IJ8Nyg1PlT6dSyM3tSjleShymIQZ1EIL3q7-YlMcYI64dMbcQlvLW2SufsdcJ_9MGR'}

The cookie's keys returned by b.cookies.all(), as you may see, are information needed about the cookies for all the urls that used them. When you set a cookie, it's assigned to the current domain and path, it has a expire date and tell if it's secure or not. But that's not for the user to provide through b.cookie.add, you just need to provide the name and value keys. In other words, each dict there is a cookie and the cookie itself is in those keys. If you can't to import an old cookie, this will work:

>>> b.cookies.add({cookie['name']: cookie['value']})

It's kinda undocumented and would be nice to add something to export and import cookie from many url at the same time, I agree. I'll add some docs on that and maybe this export/import cookies later. Thank you!

Member

douglascamata commented Feb 7, 2014

That's how Selenium cookie handler "works". In this code, for example:

>>> b = Browser()
>>> b.visit('http://google.com')
>>> b.cookies.all()
...
>>> b.cookies.all()[0]
{u'domain': u'.google.com.br',
 u'expiry': 1407542980,
 u'name': u'NID',
 u'path': u'/',
 u'secure': False,
 u'value': u'67=HPC_MzkBCnFlbZAKMFdnGhT4ZefznuCkvRabmGANtkyMAdEcGjrORYM2F_UoW9IJ8Nyg1PlT6dSyM3tSjleShymIQZ1EIL3q7-YlMcYI64dMbcQlvLW2SufsdcJ_9MGR'}

The cookie's keys returned by b.cookies.all(), as you may see, are information needed about the cookies for all the urls that used them. When you set a cookie, it's assigned to the current domain and path, it has a expire date and tell if it's secure or not. But that's not for the user to provide through b.cookie.add, you just need to provide the name and value keys. In other words, each dict there is a cookie and the cookie itself is in those keys. If you can't to import an old cookie, this will work:

>>> b.cookies.add({cookie['name']: cookie['value']})

It's kinda undocumented and would be nice to add something to export and import cookie from many url at the same time, I agree. I'll add some docs on that and maybe this export/import cookies later. Thank you!

@douglascamata

This comment has been minimized.

Show comment Hide comment
@douglascamata

douglascamata Feb 7, 2014

Member

I just made Browser.cookies.all() behave the same way for every browser and consistent with Browser.cookies.add(). Now, to get the same info that you could get before (expire date, domain, etc), you should use Browser.cookies.all(info=True) and this is in the docs. Thank you again.

Member

douglascamata commented Feb 7, 2014

I just made Browser.cookies.all() behave the same way for every browser and consistent with Browser.cookies.add(). Now, to get the same info that you could get before (expire date, domain, etc), you should use Browser.cookies.all(info=True) and this is in the docs. Thank you again.

@andrewsmedina

This comment has been minimized.

Show comment Hide comment
@andrewsmedina

andrewsmedina Feb 7, 2014

Member

@douglascamata I think that verbose is a better name than info for this parameter.

Member

andrewsmedina commented Feb 7, 2014

@douglascamata I think that verbose is a better name than info for this parameter.

@douglascamata

This comment has been minimized.

Show comment Hide comment
@douglascamata

douglascamata Feb 7, 2014

Member

@andrewsmedina gonna fix it now and another thing that I left untested (sorry)

Member

douglascamata commented Feb 7, 2014

@andrewsmedina gonna fix it now and another thing that I left untested (sorry)

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