Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error overwriting cookie when secure=True, samesite='none' set #417

Open
kenrobbins opened this issue Aug 25, 2020 · 3 comments
Open

Error overwriting cookie when secure=True, samesite='none' set #417

kenrobbins opened this issue Aug 25, 2020 · 3 comments

Comments

@kenrobbins
Copy link

kenrobbins commented Aug 25, 2020

(using webob-1.8.6, py36 and py37)

In [1]: import webob.response                                                                                                                                                                                                                 

In [2]: r = webob.response.Response()                                                                                                                                                                                                         

In [3]: r.set_cookie("foo", "val", overwrite=True, secure=True, samesite='none')                                                                                                                                                              

In [4]: r.set_cookie("bar", "val", overwrite=True, secure=True, samesite='none')                                                                                                                                                              

In [5]: r.set_cookie("bar", "val1", overwrite=True, secure=True, samesite='none')                                                                                                                                                              
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-c3dfd44d63d1> in <module>
----> 1 r.set_cookie("bar", "val", overwrite=True, secure=True, samesite='none')

/opt/webapp/userweb/lib/python3.6/site-packages/webob/response.py in set_cookie(self, name, value, max_age, path, domain, secure, httponly, comment, expires, overwrite, samesite)
   1039 
   1040         if overwrite:
-> 1041             self.unset_cookie(name, strict=False)
   1042 
   1043         # If expires is set, but not max_age we set max_age to expires

/opt/webapp/userweb/lib/python3.6/site-packages/webob/response.py in unset_cookie(self, name, strict)
   1087             del self.headers['Set-Cookie']
   1088             for m in cookies.values():
-> 1089                 self.headerlist.append(('Set-Cookie', m.serialize()))
   1090         elif strict:
   1091             raise KeyError("No cookie has been set with the name %r" % name)

/opt/webapp/userweb/lib/python3.6/site-packages/webob/cookies.py in serialize(self, full)
    297                 if not self.secure and self.samesite.lower() == b"none":
    298                     raise ValueError(
--> 299                         "Incompatible cookie attributes: "
    300                         "when the samesite equals 'none', then the secure must be True"
    301                     )

ValueError: Incompatible cookie attributes: when the samesite equals 'none', then the secure must be True
In [19]: r = webob.response.Response()                                                                                                                                                                                                        

In [20]: r.set_cookie("bar", "val", overwrite=True, secure=True, samesite='lax')                                                                                                                                                              

In [21]: r.set_cookie("foo", "val", overwrite=True, secure=True, samesite='lax')                                                                                                                                                              

In [22]: r.headerlist                                                                                                                                                                                                                         
Out[22]: 
[('Content-Type', 'text/html; charset=UTF-8'),
 ('Content-Length', '0'),
 ('Set-Cookie', 'bar=val; Path=/; secure; SameSite=lax'),
 ('Set-Cookie', 'foo=val; Path=/; secure; SameSite=lax')]

In [23]: r.set_cookie("bar", "val", overwrite=True, secure=True, samesite='lax')                                                                                                                                                              

In [24]: r.headerlist                                                                                                                                                                                                                         
Out[24]: 
[('Content-Type', 'text/html; charset=UTF-8'),
 ('Content-Length', '0'),
 ('Set-Cookie', 'foo=val; Path=/; SameSite=lax'),
 ('Set-Cookie', 'bar=val; Path=/; secure; SameSite=lax')]
In [26]: list(webob.cookies._parse_cookie("foo=val; Path=/; secure; SameSite=none"))                                                                                                                                                          
Out[26]: [(b'foo', b'val'), (b'Path', b'/'), (b'SameSite', b'none')]

It seems like when overwrite=True in set_cookie, the unset_cookie function loads the existing Set-Cookie headers without any of the equal-signless cookie attributes (HttpOnly and Secure).

So if you have an existing Set-Cookie header with SameSite=none and Secure, the Secure gets dropped and then on re-serialization, it raises a ValueError. Or if you don't run into that exception, it silently drops the HttpOnly and Secure flags.

@digitalresistor
Copy link
Member

Ah, cookie handling... my nemesis strikes again.

@kenrobbins
Copy link
Author

I worked around it by unsetting any Set-Cookie headers before calling set_cookie with overwrite=False.

def unset_cookie(response, name):
    search_name = "%s=" % name

    set_cookies = response.headers.getall('Set-Cookie')
    kept_cookies = [c for c in set_cookies if not c.startswith(search_name)]

    if len(kept_cookies) != len(set_cookies):
        del response.headers['Set-Cookie']
        for c in kept_cookies:
            response.headerlist.append(('Set-Cookie', c))

@digitalresistor
Copy link
Member

Sorry for not getting back to it sooner @kenrobbins, glad you have a work-around for right now.

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

No branches or pull requests

2 participants