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

Fix strict cookie policy. #185

Closed fschulze closed 7 years ago

fschulze commented 7 years ago

The zope.testbrowser tests are affected by this, see https://github.com/zopefoundation/zope.testbrowser/commit/8e3c130778320650076b57f956fe55907cc1c784 and https://travis-ci.org/zopefoundation/zope.testbrowser/jobs/226375649.

The commit b35063f1bc6f6b0cd92b68c56838f385b56e8d00 added a shortcut that breaks strict cookie policies. If one uses a trailing dot in HTTP_HOST when the domain is not a FQDN, then it works without the shortcut and the strict policy works as well.

@gawel since you made the commit, could you take a look at this?

gawel commented 7 years ago

This may introduce a bug in django-webtest since the default HTTP_HOST has no trailing dot https://github.com/django-webtest/django-webtest/blob/master/django_webtest/__init__.py#L56

Not sure django is able to generate correct urls if I add one...

fschulze commented 7 years ago

With no trailing dot I get [hostname].local, not sure where that comes from, but if it's standard, we could check for that.

gawel commented 7 years ago

As I remember I got that with django too. That's why I choose to match if the string starts by the host name. Don't know where that .local come from too.

gawel commented 7 years ago

Django remove the trailing dot if any. But only since 1.11: https://github.com/django/django/blob/1.11/django/http/request.py#L558 django-webtest is currently supporting 1.8+.

gawel commented 7 years ago

This works for me (eg: doesn't break django-webtest's tests):

class CookiePolicy(http_cookiejar.DefaultCookiePolicy):
    """A subclass of DefaultCookiePolicy to allow cookie set for
    Domain=localhost."""

    def return_ok_domain(self, cookie, request):
        host = request.origin_req_host
        if cookie.domain in ('.' + host, host + '.local'):
            return True
        return http_cookiejar.DefaultCookiePolicy.return_ok_domain(
            self, cookie, request)

    def set_ok_domain(self, cookie, request):
        host = request.origin_req_host
        if cookie.domain in ('.' + host, host + '.local'):
            return True
        return http_cookiejar.DefaultCookiePolicy.set_ok_domain(
            self, cookie, request)
fschulze commented 7 years ago

Does it work for you the way I changed it now?

gawel commented 7 years ago

Yes, looks good. Thanks