#3788: more tests for http.cookies, now at 95% coverage. Also bring coding style in the module up to PEP 8, where it does not break backwards compatibility.

2 files changed, 201 insertions, 127 deletions

diff --git a/Lib/http/cookies.py b/Lib/http/cookies.py
index da30e25..c36c494 100644
--- a/Lib/http/cookies.py
+++ b/Lib/http/cookies.py
@@ -46,7 +46,7 @@ At the moment, this is the only documentation.
 The Basics
 ----------
 
-Importing is easy..
+Importing is easy...
 
    >>> from http import cookies
 
@@ -127,19 +127,14 @@ the value to a string, when the values are set dictionary-style.
    'Set-Cookie: number=7\r\nSet-Cookie: string=seven'
 
 Finis.
-"""  #"
-#     ^
-#     |----helps out font-lock
+"""
 
 #
 # Import our required modules
 #
+import re
 import string
 
-from pickle import dumps, loads
-
-import re, warnings
-
 __all__ = ["CookieError", "BaseCookie", "SimpleCookie"]
 
 _nulljoin = ''.join
@@ -235,7 +230,7 @@ def _quote(str, LegalChars=_LegalChars):
     if all(c in LegalChars for c in str):
         return str
     else:
-        return '"' + _nulljoin( map(_Translator.get, str, str) ) + '"'
+        return '"' + _nulljoin(map(_Translator.get, str, str)) + '"'
 
 
 _OctalPatt = re.compile(r"\\[0-3][0-7][0-7]")
@@ -244,7 +239,7 @@ _QuotePatt = re.compile(r"[\\].")
 def _unquote(str):
     # If there aren't any doublequotes,
     # then there can't be any special characters.  See RFC 2109.
-    if  len(str) < 2:
+    if len(str) < 2:
         return str
     if str[0] != '"' or str[-1] != '"':
         return str
@@ -263,31 +258,32 @@ def _unquote(str):
     n = len(str)
     res = []
     while 0 <= i < n:
-        Omatch = _OctalPatt.search(str, i)
-        Qmatch = _QuotePatt.search(str, i)
-        if not Omatch and not Qmatch:              # Neither matched
+        o_match = _OctalPatt.search(str, i)
+        q_match = _QuotePatt.search(str, i)
+        if not o_match and not q_match:              # Neither matched
             res.append(str[i:])
             break
         # else:
         j = k = -1
-        if Omatch: j = Omatch.start(0)
-        if Qmatch: k = Qmatch.start(0)
-        if Qmatch and ( not Omatch or k < j ):     # QuotePatt matched
+        if o_match:
+            j = o_match.start(0)
+        if q_match:
+            k = q_match.start(0)
+        if q_match and (not o_match or k < j):     # QuotePatt matched
             res.append(str[i:k])
             res.append(str[k+1])
-            i = k+2
+            i = k + 2
         else:                                      # OctalPatt matched
             res.append(str[i:j])
-            res.append( chr( int(str[j+1:j+4], 8) ) )
-            i = j+4
+            res.append(chr(int(str[j+1:j+4], 8)))
+            i = j + 4
     return _nulljoin(res)
 
-# The _getdate() routine is used to set the expiration time in
-# the cookie's HTTP header.      By default, _getdate() returns the
-# current time in the appropriate "expires" format for a
-# Set-Cookie header.     The one optional argument is an offset from
-# now, in seconds.      For example, an offset of -3600 means "one hour ago".
-# The offset may be a floating point number.
+# The _getdate() routine is used to set the expiration time in the cookie's HTTP
+# header.  By default, _getdate() returns the current time in the appropriate
+# "expires" format for a Set-Cookie header.  The one optional argument is an
+# offset from now, in seconds.  For example, an offset of -3600 means "one hour
+# ago".  The offset may be a floating point number.
 #
 
 _weekdayname = ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun']
@@ -305,7 +301,7 @@ def _getdate(future=0, weekdayname=_weekdayname, monthname=_monthname):
 
 
 class Morsel(dict):
-    """A class to hold ONE key,value pair.
+    """A class to hold ONE (key, value) pair.
 
     In a cookie, each such pair may have several attributes, so this class is
     used to keep the attributes associated with the appropriate key,value pair.
@@ -326,23 +322,24 @@ class Morsel(dict):
     # This dictionary provides a mapping from the lowercase
     # variant on the left to the appropriate traditional
     # formatting on the right.
-    _reserved = { "expires" : "expires",
-                   "path"        : "Path",
-                   "comment" : "Comment",
-                   "domain"      : "Domain",
-                   "max-age" : "Max-Age",
-                   "secure"      : "secure",
-                   "httponly"  : "httponly",
-                   "version" : "Version",
-                   }
+    _reserved = {
+        "expires"  : "expires",
+        "path"     : "Path",
+        "comment"  : "Comment",
+        "domain"   : "Domain",
+        "max-age"  : "Max-Age",
+        "secure"   : "secure",
+        "httponly" : "httponly",
+        "version"  : "Version",
+    }
 
     def __init__(self):
         # Set defaults
         self.key = self.value = self.coded_value = None
 
         # Set default attributes
-        for K in self._reserved:
-            dict.__setitem__(self, K, "")
+        for key in self._reserved:
+            dict.__setitem__(self, key, "")
 
     def __setitem__(self, K, V):
         K = K.lower()
@@ -362,18 +359,18 @@ class Morsel(dict):
             raise CookieError("Illegal key value: %s" % key)
 
         # It's a good key, so save it.
-        self.key                 = key
-        self.value               = val
-        self.coded_value         = coded_val
+        self.key = key
+        self.value = val
+        self.coded_value = coded_val
 
-    def output(self, attrs=None, header = "Set-Cookie:"):
-        return "%s %s" % ( header, self.OutputString(attrs) )
+    def output(self, attrs=None, header="Set-Cookie:"):
+        return "%s %s" % (header, self.OutputString(attrs))
 
     __str__ = output
 
     def __repr__(self):
         return '<%s: %s=%s>' % (self.__class__.__name__,
-                                self.key, repr(self.value) )
+                                self.key, repr(self.value))
 
     def js_output(self, attrs=None):
         # Print javascript
@@ -383,34 +380,36 @@ class Morsel(dict):
         document.cookie = \"%s\";
         // end hiding -->
         </script>
-        """ % ( self.OutputString(attrs).replace('"',r'\"'))
+        """ % (self.OutputString(attrs).replace('"', r'\"'))
 
     def OutputString(self, attrs=None):
         # Build up our result
         #
         result = []
-        RA = result.append
+        append = result.append
 
         # First, the key=value pair
-        RA("%s=%s" % (self.key, self.coded_value))
+        append("%s=%s" % (self.key, self.coded_value))
 
         # Now add any defined attributes
         if attrs is None:
             attrs = self._reserved
         items = sorted(self.items())
-        for K,V in items:
-            if V == "": continue
-            if K not in attrs: continue
-            if K == "expires" and type(V) == type(1):
-                RA("%s=%s" % (self._reserved[K], _getdate(V)))
-            elif K == "max-age" and type(V) == type(1):
-                RA("%s=%d" % (self._reserved[K], V))
-            elif K == "secure":
-                RA(str(self._reserved[K]))
-            elif K == "httponly":
-                RA(str(self._reserved[K]))
+        for key, value in items:
+            if value == "":
+                continue
+            if key not in attrs:
+                continue
+            if key == "expires" and isinstance(value, int):
+                append("%s=%s" % (self._reserved[key], _getdate(value)))
+            elif key == "max-age" and isinstance(value, int):
+                append("%s=%d" % (self._reserved[key], value))
+            elif key == "secure":
+                append(str(self._reserved[key]))
+            elif key == "httponly":
+                append(str(self._reserved[key]))
             else:
-                RA("%s=%s" % (self._reserved[K], V))
+                append("%s=%s" % (self._reserved[key], value))
 
         # Return the result
         return _semispacejoin(result)
@@ -426,24 +425,23 @@ class Morsel(dict):
 #
 
 _LegalCharsPatt  = r"[\w\d!#%&'~_`><@,:/\$\*\+\-\.\^\|\)\(\?\}\{\=]"
-_CookiePattern = re.compile(
-    r"(?x)"                       # This is a Verbose pattern
-    r"(?P<key>"                   # Start of group 'key'
-    ""+ _LegalCharsPatt +"+?"     # Any word of at least one letter, nongreedy
-    r")"                          # End of group 'key'
-    r"\s*=\s*"                    # Equal Sign
-    r"(?P<val>"                   # Start of group 'val'
-    r'"(?:[^\\"]|\\.)*"'            # Any doublequoted string
-    r"|"                            # or
-    ""+ _LegalCharsPatt +"*"        # Any word or empty string
-    r")"                          # End of group 'val'
-    r"\s*;?"                      # Probably ending in a semi-colon
-    , re.ASCII)                   # May be removed if safe.
-
-
-# At long last, here is the cookie class.
-#   Using this class is almost just like using a dictionary.
-# See this module's docstring for example usage.
+_CookiePattern = re.compile(r"""
+    (?x)                           # This is a verbose pattern
+    (?P<key>                       # Start of group 'key'
+    """ + _LegalCharsPatt + r"""+?   # Any word of at least one letter
+    )                              # End of group 'key'
+    \s*=\s*                        # Equal Sign
+    (?P<val>                       # Start of group 'val'
+    "(?:[^\\"]|\\.)*"                # Any doublequoted string
+    |                                # or
+    """ + _LegalCharsPatt + r"""*    # Any word or empty string
+    )                              # End of group 'val'
+    \s*;?                          # Probably ending in a semi-colon
+    """, re.ASCII)                 # May be removed if safe.
+
+
+# At long last, here is the cookie class.  Using this class is almost just like
+# using a dictionary.  See this module's docstring for example usage.
 #
 class BaseCookie(dict):
     """A container class for a set of Morsels."""
@@ -467,7 +465,8 @@ class BaseCookie(dict):
         return strval, strval
 
     def __init__(self, input=None):
-        if input: self.load(input)
+        if input:
+            self.load(input)
 
     def __set(self, key, real_value, coded_value):
         """Private method for setting a cookie's value"""
@@ -484,25 +483,25 @@ class BaseCookie(dict):
         """Return a string suitable for HTTP."""
         result = []
         items = sorted(self.items())
-        for K,V in items:
-            result.append( V.output(attrs, header) )
+        for key, value in items:
+            result.append(value.output(attrs, header))
         return sep.join(result)
 
     __str__ = output
 
     def __repr__(self):
-        L = []
+        l = []
         items = sorted(self.items())
-        for K,V in items:
-            L.append( '%s=%s' % (K,repr(V.value) ) )
-        return '<%s: %s>' % (self.__class__.__name__, _spacejoin(L))
+        for key, value in items:
+            l.append('%s=%s' % (key, repr(value.value)))
+        return '<%s: %s>' % (self.__class__.__name__, _spacejoin(l))
 
     def js_output(self, attrs=None):
         """Return a string suitable for JavaScript."""
         result = []
         items = sorted(self.items())
-        for K,V in items:
-            result.append( V.js_output(attrs) )
+        for key, value in items:
+            result.append(value.js_output(attrs))
         return _nulljoin(result)
 
     def load(self, rawdata):
@@ -511,15 +510,15 @@ class BaseCookie(dict):
         is equivalent to calling:
             map(Cookie.__setitem__, d.keys(), d.values())
         """
-        if type(rawdata) == type(""):
-            self.__ParseString(rawdata)
+        if isinstance(rawdata, str):
+            self.__parse_string(rawdata)
         else:
             # self.update() wouldn't call our custom __setitem__
-            for k, v in rawdata.items():
-                self[k] = v
+            for key, value in rawdata.items():
+                self[key] = value
         return
 
-    def __ParseString(self, str, patt=_CookiePattern):
+    def __parse_string(self, str, patt=_CookiePattern):
         i = 0            # Our starting point
         n = len(str)     # Length of string
         M = None         # current morsel
@@ -527,25 +526,27 @@ class BaseCookie(dict):
         while 0 <= i < n:
             # Start looking for a cookie
             match = patt.search(str, i)
-            if not match: break          # No more cookies
+            if not match:
+                # No more cookies
+                break
 
-            K,V = match.group("key"), match.group("val")
+            key, value = match.group("key"), match.group("val")
             i = match.end(0)
 
             # Parse the key, value in case it's metainfo
-            if K[0] == "$":
+            if key[0] == "$":
                 # We ignore attributes which pertain to the cookie
                 # mechanism as a whole.  See RFC 2109.
                 # (Does anyone care?)
                 if M:
-                    M[ K[1:] ] = V
-            elif K.lower() in Morsel._reserved:
+                    M[key[1:]] = value
+            elif key.lower() in Morsel._reserved:
                 if M:
-                    M[ K ] = _unquote(V)
+                    M[key] = _unquote(value)
             else:
-                rval, cval = self.value_decode(V)
-                self.__set(K, rval, cval)
-                M = self[K]
+                rval, cval = self.value_decode(value)
+                self.__set(key, rval, cval)
+                M = self[key]
 
 
 class SimpleCookie(BaseCookie):
@@ -556,16 +557,8 @@ class SimpleCookie(BaseCookie):
     received from HTTP are kept as strings.
     """
     def value_decode(self, val):
-        return _unquote( val ), val
+        return _unquote(val), val
+
     def value_encode(self, val):
         strval = str(val)
-        return strval, _quote( strval )
-
-###########################################################
-
-def _test():
-    import doctest, http.cookies
-    return doctest.testmod(http.cookies)
-
-if __name__ == "__main__":
-    _test()
+        return strval, _quote(strval)
diff --git a/Lib/test/test_http_cookies.py b/Lib/test/test_http_cookies.py
index 4d9c9ca..43ca9f5 100644
--- a/Lib/test/test_http_cookies.py
+++ b/Lib/test/test_http_cookies.py
@@ -19,24 +19,21 @@ class CookieTests(unittest.TestCase):
 
     def test_basic(self):
         cases = [
-            { 'data': 'chips=ahoy; vienna=finger',
-              'dict': {'chips':'ahoy', 'vienna':'finger'},
-              'repr': "<SimpleCookie: chips='ahoy' vienna='finger'>",
-              'output': 'Set-Cookie: chips=ahoy\nSet-Cookie: vienna=finger',
-            },
-
-            { 'data': 'keebler="E=mc2; L=\\"Loves\\"; fudge=\\012;"',
-              'dict': {'keebler' : 'E=mc2; L="Loves"; fudge=\012;'},
-              'repr': '''<SimpleCookie: keebler='E=mc2; L="Loves"; fudge=\\n;'>''',
-              'output': 'Set-Cookie: keebler="E=mc2; L=\\"Loves\\"; fudge=\\012;"',
-            },
+            {'data': 'chips=ahoy; vienna=finger',
+             'dict': {'chips':'ahoy', 'vienna':'finger'},
+             'repr': "<SimpleCookie: chips='ahoy' vienna='finger'>",
+             'output': 'Set-Cookie: chips=ahoy\nSet-Cookie: vienna=finger'},
+
+            {'data': 'keebler="E=mc2; L=\\"Loves\\"; fudge=\\012;"',
+             'dict': {'keebler' : 'E=mc2; L="Loves"; fudge=\012;'},
+             'repr': '''<SimpleCookie: keebler='E=mc2; L="Loves"; fudge=\\n;'>''',
+             'output': 'Set-Cookie: keebler="E=mc2; L=\\"Loves\\"; fudge=\\012;"'},
 
             # Check illegal cookies that have an '=' char in an unquoted value
-            { 'data': 'keebler=E=mc2',
-              'dict': {'keebler' : 'E=mc2'},
-              'repr': "<SimpleCookie: keebler='E=mc2'>",
-              'output': 'Set-Cookie: keebler=E=mc2',
-            }
+            {'data': 'keebler=E=mc2',
+             'dict': {'keebler' : 'E=mc2'},
+             'repr': "<SimpleCookie: keebler='E=mc2'>",
+             'output': 'Set-Cookie: keebler=E=mc2'},
         ]
 
         for case in cases:
@@ -72,6 +69,26 @@ class CookieTests(unittest.TestCase):
         </script>
         """)
 
+    def test_special_attrs(self):
+        # 'expires'
+        C = cookies.SimpleCookie('Customer="WILE_E_COYOTE"')
+        C['Customer']['expires'] = 0
+        # can't test exact output, it always depends on current date/time
+        self.assertTrue(C.output().endswith('GMT'))
+
+        # 'max-age'
+        C = cookies.SimpleCookie('Customer="WILE_E_COYOTE"')
+        C['Customer']['max-age'] = 10
+        self.assertEqual(C.output(),
+                         'Set-Cookie: Customer="WILE_E_COYOTE"; Max-Age=10')
+
+        # others
+        C = cookies.SimpleCookie('Customer="WILE_E_COYOTE"')
+        C['Customer']['secure'] = True
+        C['Customer']['httponly'] = True
+        self.assertEqual(C.output(),
+            'Set-Cookie: Customer="WILE_E_COYOTE"; httponly; secure')
+
     def test_quoted_meta(self):
         # Try cookie with quoted meta-data
         C = cookies.SimpleCookie()
@@ -80,8 +97,72 @@ class CookieTests(unittest.TestCase):
         self.assertEqual(C['Customer']['version'], '1')
         self.assertEqual(C['Customer']['path'], '/acme')
 
+        self.assertEqual(C.output(['path']),
+                         'Set-Cookie: Customer="WILE_E_COYOTE"; Path=/acme')
+        self.assertEqual(C.js_output(), r"""
+        <script type="text/javascript">
+        <!-- begin hiding
+        document.cookie = "Customer=\"WILE_E_COYOTE\"; Path=/acme; Version=1";
+        // end hiding -->
+        </script>
+        """)
+        self.assertEqual(C.js_output(['path']), r"""
+        <script type="text/javascript">
+        <!-- begin hiding
+        document.cookie = "Customer=\"WILE_E_COYOTE\"; Path=/acme";
+        // end hiding -->
+        </script>
+        """)
+
+class MorselTests(unittest.TestCase):
+    """Tests for the Morsel object."""
+
+    def test_reserved_keys(self):
+        M = cookies.Morsel()
+        # tests valid and invalid reserved keys for Morsels
+        for i in M._reserved:
+            # Test that all valid keys are reported as reserved and set them
+            self.assertTrue(M.isReservedKey(i))
+            M[i] = '%s_value' % i
+        for i in M._reserved:
+            # Test that valid key values come out fine
+            self.assertEqual(M[i], '%s_value' % i)
+        for i in "the holy hand grenade".split():
+            # Test that invalid keys raise CookieError
+            self.assertRaises(cookies.CookieError,
+                              M.__setitem__, i, '%s_value' % i)
+
+    def test_setter(self):
+        M = cookies.Morsel()
+        # tests the .set method to set keys and their values
+        for i in M._reserved:
+            # Makes sure that all reserved keys can't be set this way
+            self.assertRaises(cookies.CookieError,
+                              M.set, i, '%s_value' % i, '%s_value' % i)
+        for i in "thou cast _the- !holy! ^hand| +*grenade~".split():
+            # Try typical use case. Setting decent values.
+            # Check output and js_output.
+            M['path'] = '/foo' # Try a reserved key as well
+            M.set(i, "%s_val" % i, "%s_coded_val" % i)
+            self.assertEqual(
+                M.output(),
+                "Set-Cookie: %s=%s; Path=/foo" % (i, "%s_coded_val" % i))
+            expected_js_output = """
+        <script type="text/javascript">
+        <!-- begin hiding
+        document.cookie = "%s=%s; Path=/foo";
+        // end hiding -->
+        </script>
+        """ % (i, "%s_coded_val" % i)
+            self.assertEqual(M.js_output(), expected_js_output)
+        for i in ["foo bar", "foo@bar"]:
+            # Try some illegal characters
+            self.assertRaises(cookies.CookieError,
+                              M.set, i, '%s_value' % i, '%s_value' % i)
+
+
 def test_main():
-    run_unittest(CookieTests)
+    run_unittest(CookieTests, MorselTests)
     run_doctest(cookies)
 
 if __name__ == '__main__':