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

Invalid testapp.cookies after testapp.set_cookie() #171

Open
marc1n opened this issue Jan 18, 2017 · 13 comments
Open

Invalid testapp.cookies after testapp.set_cookie() #171

marc1n opened this issue Jan 18, 2017 · 13 comments

Comments

@marc1n
Copy link

marc1n commented Jan 18, 2017

Dictionary testapp.cookies has invalid value after call testapp.set_cookie():

>>> from webtest import TestApp
>>> from webtest.debugapp import debug_app
>>> testapp = TestApp(debug_app)
>>> name, val = 'c', '123'
>>> testapp.set_cookie(name, val)
>>> c_val = testapp.cookies.get(name)
>>> val == c_val
False
>>> val
'123'
>>> c_val
'"123"'
@marc1n marc1n changed the title Invalid testapp.cookies value after testapp.set_value() Invalid testapp.cookies after testapp.set_value() Jan 18, 2017
@marc1n marc1n changed the title Invalid testapp.cookies after testapp.set_value() Invalid testapp.cookies after testapp.set_cookie() Jan 18, 2017
@gawel
Copy link
Member

gawel commented Jan 18, 2017

Its not invalid. cookies are escaped. See

value = escape_cookie_value(value)

@marc1n
Copy link
Author

marc1n commented Jan 18, 2017

testapp.cookies.get shoud return unescaped value.

@gawel
Copy link
Member

gawel commented Jan 18, 2017

I don't think so.

.get() return the stored value. If the app return an escaped cookie it should be stored like this.

@marc1n
Copy link
Author

marc1n commented Jan 18, 2017

OK. So, how to get unescaped value of cookie?

BTW. I think calling escape_cookie_value() in webtest is redundant because cookiejar escapes cookie value if needed.

@gawel
Copy link
Member

gawel commented Jan 18, 2017

There is not way to do that AFAIK. You can try to add an app.get_cookie(name, unescaped=True/False) if you need but you'll have to deal with the RFC mess

As I remember we always escape cookies because it's easier. Not sure it's really true.

Btw, why do you need this ? Your webapp don't take care of escaped cookies ?

@marc1n
Copy link
Author

marc1n commented Jan 18, 2017

Btw, why do you need this ? Your webapp don't take care of escaped cookies

No, webapp handles escaped cookies correctly.

Problem is in unittest's code:
In one place we set cookie (testapp.set_cookie()) and in an another place of the same test (before we call testapp.post()) we check cookie value (testapp.cookies.get()) - and we have problem because these values are not equal.

@riskingh
Copy link

Is there any chances this issue gets resolved or documented at this point?

@digitalresistor
Copy link
Member

Just adding quotes to the cookie is not valid according to the RFC, there are specific requirements for what may or may not be stored in a cookie. WebOb itself has a whole bunch of cookie handling, why not re-use parts of that? Especially the serialization bits that will currently warn, but in the future will raise if you attempt to set an invalid cookie?

See from here on down: https://github.com/Pylons/webob/blob/master/src/webob/cookies.py#L338

@gawel
Copy link
Member

gawel commented Nov 29, 2017

It sure is a good idea to use webob helpers. Not sure it'll be an easy task

@mmerickel
Copy link
Member

Here is some code using webob to parse the cookies in a cookiejar into a dict. This is what I'd expect TestApp.cookies to be doing for me. It's very surprising that .cookies is currently the escaped value.

from webob.cookies import Cookie

def parse_cookiejar(jar):
    cookie = Cookie(' '.join(c.name + '=' + c.value for c in jar))
    return {
        m.name.decode('latin1'): m.value.decode('latin1')
        for m in cookie.values()
    }

cookies = parse_cookiejar(testapp.cookiejar)

@mmerickel
Copy link
Member

A quick PR could just add TestApp.get_cookie as an inverse to TestApp.set_cookie but I do believe TestApp.cookies should be unescaped.

from webob.cookies import Cookie

def get_cookie(self, name, default=None):
    cookie = Cookie(' '.join(
        '%s=%s' % (c.name, c.value)
        for c in self.cookiejar
        if c.name == name
    ))
    return next(
        (m.value.decode('latin-1') for m in cookie.values()),
        default,
    )

@mmerickel
Copy link
Member

@gawel could I get some insight on an approach you'd be comfortable with so that I can make a PR? Would you prefer a get_cookie api or would you be open to fixing the .cookies attribute to contain unescaped values?

@gawel
Copy link
Member

gawel commented Jan 23, 2020

I'm open to everything. I remember that this code is old (maybe written at the webtest sprint.. 10 years ago ? :D )

The best approach is probably to replace everything you can by using webob's api.

I have no problem to break backward compats. This is why semver is for. And maybe it can be a good opportunity to remove py2 compat too.

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

No branches or pull requests

5 participants