[checkmk-commits] Fixed handling of cookies containing non-ascii characters

Module: check_mk Branch: master Commit: 03014daecb0a0bce9e307105605843bcef6b6c2a URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=03014daecb0a0b… Author: Lars Michelsen &lt;lm(a)mathias-kettner.de&gt; Date: Tue Aug 14 21:06:38 2018 +0200 Fixed handling of cookies containing non-ascii characters This issue was introduced during WSGI migration in 1.6 only. Add test to verify that <1.5 non-ascii cookie format can be handled Change-Id: I1216a3995a8556c7484a9bc84a475257e4f22fb6 --- cmk/gui/http.py | 40 ++++++++++++++++++++-------------------- tests/unit/web/test_http.py | 33 +++++++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/cmk/gui/http.py b/cmk/gui/http.py index c206423..025bc3c 100644 --- a/cmk/gui/http.py +++ b/cmk/gui/http.py @@ -28,7 +28,7 @@ import re import time -import Cookie +import werkzeug.http import cgi import cmk.gui.log as log @@ -55,10 +55,13 @@ class Request(object): self.vars = {} self.listvars = {} # for variables with more than one occurrence self.uploads = {} - self.cookies = Cookie.BaseCookie() + # TODO: To be compatible with Check_MK <1.5 handling / code base we + # prevent parse_cookie() from decoding the stuff to unicode. One bright + # day we'll switch all input stuff to be parsed to unicode, then we'll + # clean this up! + self.cookies = werkzeug.http.parse_cookie(wsgi_environ, charset=None) self._init_vars(wsgi_environ) - self.cookies.load(wsgi_environ.get("HTTP_COOKIE", {})) def _init_vars(self, wsgi_environ): @@ -184,7 +187,7 @@ class Request(object): def cookie(self, varname, deflt=None): """Return either the value of the cookie provided by the client, the given deflt value or None""" try: - return self.cookies[varname].value + return self.cookies[varname] except: return deflt @@ -322,25 +325,22 @@ class Response(object): def set_cookie(self, varname, value, expires=None): """Send the given cookie to the client with the response""" - c = Cookie.BaseCookie() - c[varname] = value.encode("utf-8") if type(value) == unicode else "%s" % value - - # Use cookie for all URLs of the host (to make cross site use possible) - c[varname]["path"] = "/" - - # Tell client not to use the cookie within javascript - c[varname]["httponly"] = True - - # Tell client to only use the cookie for SSL connections - if self._request.is_ssl_request: - c[varname]["secure"] = True - if expires is not None: assert type(expires) == int - c[varname]["expires"] = expires - self.set_http_header("Set-Cookie", c.output(header="").lstrip(), - prevent_duplicate=False) + cookie_header = werkzeug.http.dump_cookie( + varname, + value, + # Use cookie for all URLs of the host (to make cross site use possible) + path="/", + # Tell client not to use the cookie within javascript + httponly=True, + # Tell client to only use the cookie for SSL connections + secure=self._request.is_ssl_request, + expires=expires, + ) + + self.set_http_header("Set-Cookie", cookie_header, prevent_duplicate=False) def del_cookie(self, varname): diff --git a/tests/unit/web/test_http.py b/tests/unit/web/test_http.py index 823a0e0..196175d 100644 --- a/tests/unit/web/test_http.py +++ b/tests/unit/web/test_http.py @@ -32,7 +32,7 @@ def test_response_set_cookie(register_builtin_html): html.response.set_cookie("auth_SITE", "user:123456:abcdefg") assert html.response.headers[-1] == \ - ("Set-Cookie", "auth_SITE=user:123456:abcdefg; httponly; Path=/") + ("Set-Cookie", "auth_SITE=user:123456:abcdefg; HttpOnly; Path=/") def test_response_set_cookie_secure(register_builtin_html, monkeypatch): @@ -42,7 +42,7 @@ def test_response_set_cookie_secure(register_builtin_html, monkeypatch): html.response.set_cookie("auth_SITE", "user:123456:abcdefg") assert html.response.headers[-1] == \ - ("Set-Cookie", "auth_SITE=user:123456:abcdefg; httponly; Path=/; secure") + ("Set-Cookie", "auth_SITE=user:123456:abcdefg; Secure; HttpOnly; Path=/") def test_response_set_cookie_expires(register_builtin_html, monkeypatch): @@ -51,7 +51,7 @@ def test_response_set_cookie_expires(register_builtin_html, monkeypatch): html.response.set_cookie("auth_SITE", "user:123456:abcdefg", expires=60) assert html.response.headers[-1] == \ - ("Set-Cookie", "auth_SITE=user:123456:abcdefg; expires=Thu, 01 Jan 1970 00:01:00 GMT; httponly; Path=/") + ("Set-Cookie", "auth_SITE=user:123456:abcdefg; Expires=Thu, 01-Jan-1970 00:01:00 GMT; HttpOnly; Path=/") def test_response_del_cookie(register_builtin_html, monkeypatch): @@ -60,4 +60,29 @@ def test_response_del_cookie(register_builtin_html, monkeypatch): html.response.del_cookie("auth_SITE") assert html.response.headers[-1] == \ - ("Set-Cookie", "auth_SITE=; expires=Wed, 31 Dec 1969 23:59:00 GMT; httponly; Path=/") + ("Set-Cookie", "auth_SITE=; Expires=Wed, 31-Dec-1969 23:59:00 GMT; HttpOnly; Path=/") + + +# User IDs in Check_MK may contain non ascii characters. When they need to be encoded, +# they are encoded in UTF-8. Since this is possible the user names in the cookies directly +# contain those UTF-8 encoded user names. +# Until we decide that distributed setups between the current version and the previous +# versions are not possible anymore we need to be able to deal with the old cookie format. +# +# We dropped the old format during 1.6 development. It would be a good time to drop the +# compatibility with the old format earliest with 1.7. +def test_pre_16_format_cookie_handling(monkeypatch): + wsgi_environ = { + # This is no complete WSGI environment. But we currently don't need more. + "wsgi.input" : "", + "SCRIPT_NAME" : "", + "HTTP_COOKIE" : "xyz=123; auth_stable=lärs:1534272374.61:1f59cac3fcd5bcc389e4f8397bed315b; abc=123" + } + + request = http.Request(wsgi_environ) + + assert isinstance(request.cookie("auth_stable"), bytes) + assert request.cookie("auth_stable") == "lärs:1534272374.61:1f59cac3fcd5bcc389e4f8397bed315b" + + assert request.has_cookie("xyz") + assert request.has_cookie("abc")