Pylons / webtest

Wraps any WSGI application and makes it easy to send test requests to that application, without starting up an HTTP server.
https://docs.pylonsproject.org/projects/webtest/en/latest/
Other
336 stars 110 forks source link

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

Open marc1n opened 7 years ago

marc1n commented 7 years ago

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"'
gawel commented 7 years ago

Its not invalid. cookies are escaped. See https://github.com/Pylons/webtest/blob/3a47668a026d6c55bef75ab2a7cf08662211355b/webtest/app.py#L234

marc1n commented 7 years ago

testapp.cookies.get shoud return unescaped value.

gawel commented 7 years ago

I don't think so.

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

marc1n commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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

digitalresistor commented 6 years ago

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 commented 6 years ago

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

mmerickel commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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.