refactor session cookie domain logic

cache result of session cookie domain
add warnings for session cookie domain issues
add changelog

CHANGES

@@ -34,6 +34,12 @@ Major release, unreleased
   type is invalid. (`#2256`_)
 - Add ``routes`` CLI command to output routes registered on the application.
   (`#2259`_)
+- Show warning when session cookie domain is a bare hostname or an IP
+  address, as these may not behave properly in some browsers, such as Chrome.
+  (`#2282`_)
+- Allow IP address as exact session cookie domain. (`#2282`_)
+- ``SESSION_COOKIE_DOMAIN`` is set if it is detected through ``SERVER_NAME``.
+  (`#2282`_)
 
 .. _#1489: https://github.com/pallets/flask/pull/1489
 .. _#1898: https://github.com/pallets/flask/pull/1898
@@ -43,6 +49,7 @@ Major release, unreleased
 .. _#2254: https://github.com/pallets/flask/pull/2254
 .. _#2256: https://github.com/pallets/flask/pull/2256
 .. _#2259: https://github.com/pallets/flask/pull/2259
+.. _#2282: https://github.com/pallets/flask/pull/2282
 
 Version 0.12.1
 --------------

flask/helpers.py

@@ -10,6 +10,7 @@
 """
 
 import os
+import socket
 import sys
 import pkgutil
 import posixpath
@@ -977,22 +978,23 @@ def total_seconds(td):
     """
     return td.days * 60 * 60 * 24 + td.seconds
 
-def is_ip(ip):
-    """Returns the if the string received is an IP or not.
 
-    :param string: the string to check if it an IP or not
-    :param var_name: the name of the string that is being checked
+def is_ip(value):
+    """Determine if the given string is an IP address.
 
-    :returns: True if string is an IP, False if not
-    :rtype: boolean
+    :param value: value to check
+    :type value: str
+
+    :return: True if string is an IP address
+    :rtype: bool
     """
-    import socket
 
     for family in (socket.AF_INET, socket.AF_INET6):
         try:
-            socket.inet_pton(family, ip)
+            socket.inet_pton(family, value)
         except socket.error:
             pass
         else:
             return True
+
     return False

flask/sessions.py

@@ -11,7 +11,7 @@
 
 import uuid
 import hashlib
-from warnings import warn
+import warnings
 from base64 import b64encode, b64decode
 from datetime import datetime
 from werkzeug.http import http_date, parse_date
@@ -201,29 +201,61 @@ class SessionInterface(object):
         return isinstance(obj, self.null_session_class)
 
     def get_cookie_domain(self, app):
-        """Helpful helper method that returns the cookie domain that should
-        be used for the session cookie if session cookies are used.
+        """Returns the domain that should be set for the session cookie.
+        
+        Uses ``SESSION_COOKIE_DOMAIN`` if it is configured, otherwise
+        falls back to detecting the domain based on ``SERVER_NAME``.
+        
+        Once detected (or if not set at all), ``SESSION_COOKIE_DOMAIN`` is
+        updated to avoid re-running the logic.
         """
-        if app.config['SESSION_COOKIE_DOMAIN'] is not None:
-            return app.config['SESSION_COOKIE_DOMAIN']
-        if app.config['SERVER_NAME'] is not None:
-            # chop off the port which is usually not supported by browsers
-            rv = '.' + app.config['SERVER_NAME'].rsplit(':', 1)[0]
 
-            # Google chrome does not like cookies set to .localhost, so
-            # we just go with no domain then.  Flask documents anyways that
-            # cross domain cookies need a fully qualified domain name
-            if rv == '.localhost':
-                rv = None
+        rv = app.config['SESSION_COOKIE_DOMAIN']
 
-            # If we infer the cookie domain from the server name we need
-            # to check if we are in a subpath.  In that case we can't
-            # set a cross domain cookie.
+        # set explicitly, or cached from SERVER_NAME detection
+        # if False, return None
         if rv is not None:
-                path = self.get_cookie_path(app)
-                if path != '/':
-                    rv = rv.lstrip('.')
+            return rv if rv else None
 
+        rv = app.config['SERVER_NAME']
+
+        # server name not set, cache False to return none next time
+        if not rv:
+            app.config['SESSION_COOKIE_DOMAIN'] = False
+            return None
+
+        # chop off the port which is usually not supported by browsers
+        # remove any leading '.' since we'll add that later
+        rv = rv.rsplit(':', 1)[0].lstrip('.')
+
+        if '.' not in rv:
+            # Chrome doesn't allow names without a '.'
+            # this should only come up with localhost
+            # hack around this by not setting the name, and show a warning
+            warnings.warn(
+                '"{rv}" is not a valid cookie domain, it must contain a ".".'
+                ' Add an entry to your hosts file, for example'
+                ' "{rv}.localdomain", and use that instead.'.format(rv=rv)
+            )
+            app.config['SESSION_COOKIE_DOMAIN'] = False
+            return None
+
+        ip = is_ip(rv)
+
+        if ip:
+            warnings.warn(
+                'The session cookie domain is an IP address. This may not work'
+                ' as intended in some browsers. Add an entry to your hosts'
+                ' file, for example "localhost.localdomain", and use that'
+                ' instead.'
+            )
+
+        # if this is not an ip and app is mounted at the root, allow subdomain
+        # matching by adding a '.' prefix
+        if self.get_cookie_path(app) == '/' and not ip:
+            rv = '.' + rv
+
+        app.config['SESSION_COOKIE_DOMAIN'] = rv
         return rv
 
     def get_cookie_path(self, app):
@@ -337,9 +369,6 @@ class SecureCookieSessionInterface(SessionInterface):
 
     def save_session(self, app, session, response):
         domain = self.get_cookie_domain(app)
-        if domain is not None:
-            if is_ip(domain):
-                warnings.warn("IP introduced in SESSION_COOKIE_DOMAIN", RuntimeWarning)
         path = self.get_cookie_path(app)
 
         # Delete case.  If there is no session we bail early.

tests/test_basic.py

@@ -351,6 +351,42 @@ def test_session_using_session_settings():
     assert 'httponly' not in cookie
 
 
+def test_session_localhost_warning(recwarn):
+    app = flask.Flask(__name__)
+    app.config.update(
+        SECRET_KEY='testing',
+        SERVER_NAME='localhost:5000',
+    )
+
+    @app.route('/')
+    def index():
+        flask.session['testing'] = 42
+        return 'testing'
+
+    rv = app.test_client().get('/', 'http://localhost:5000/')
+    assert 'domain' not in rv.headers['set-cookie'].lower()
+    w = recwarn.pop(UserWarning)
+    assert '"localhost" is not a valid cookie domain' in str(w.message)
+
+
+def test_session_ip_warning(recwarn):
+    app = flask.Flask(__name__)
+    app.config.update(
+        SECRET_KEY='testing',
+        SERVER_NAME='127.0.0.1:5000',
+    )
+
+    @app.route('/')
+    def index():
+        flask.session['testing'] = 42
+        return 'testing'
+
+    rv = app.test_client().get('/', 'http://127.0.0.1:5000/')
+    assert 'domain=127.0.0.1' in rv.headers['set-cookie'].lower()
+    w = recwarn.pop(UserWarning)
+    assert 'cookie domain is an IP' in str(w.message)
+
+
 def test_missing_session():
     app = flask.Flask(__name__)