Pylons / Pylons/webob

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

Open
#417 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

sprintable
Dominant language
Python
Stars
443
Forks
206
PR merge metrics
No merged PRs in 30d

Description

(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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in webob/response.py at set_cookie and unset_cookie, then inspect webob/cookies.py, especially _parse_cookie and Cookie.serialize. Reproduce the overwrite=True examples and ensure existing Secure and HttpOnly attributes survive re-serialization, including SameSite=none without raising ValueError.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.