Issue #2211: Updated the implementation of the http.cookies.Morsel class.

Setting attributes key, value and coded_value directly now is deprecated.
update() and setdefault() now transform and check keys.  Comparing for
equality now takes into account attributes key, value and coded_value.
copy() now returns a Morsel, not a dict.  repr() now contains all attributes.
Optimized checking keys and quoting values.  Added new tests.
Original patch by Demian Brecht.

5 files changed, 299 insertions, 86 deletions

diff --git a/Doc/library/http.cookies.rst b/Doc/library/http.cookies.rst
index 646f2e8..46c3bbd 100644
--- a/Doc/library/http.cookies.rst
+++ b/Doc/library/http.cookies.rst
@@ -148,16 +148,28 @@ Morsel Objects
 
    The value of the cookie.
 
+   .. deprecated:: 3.5
+      Setting :attr:`~Morsel.value` directly has been deprecated in favour of
+      using :func:`~Morsel.set`
+
 
 .. attribute:: Morsel.coded_value
 
    The encoded value of the cookie --- this is what should be sent.
 
+   .. deprecated:: 3.5
+      Setting :attr:`~Morsel.coded_value` directly has been deprecated in
+      favour of using :func:`~Morsel.set`
+
 
 .. attribute:: Morsel.key
 
    The name of the cookie.
 
+   .. deprecated:: 3.5
+      Setting :attr:`~Morsel.key` directly has been deprecated in
+      favour of using :func:`~Morsel.set`
+
 
 .. method:: Morsel.set(key, value, coded_value)
 
diff --git a/Doc/whatsnew/3.5.rst b/Doc/whatsnew/3.5.rst
index 21fafd0..a70f0b8 100644
--- a/Doc/whatsnew/3.5.rst
+++ b/Doc/whatsnew/3.5.rst
@@ -512,6 +512,12 @@ Deprecated Python modules, functions and methods
   ``True``, but this default is deprecated.  Specify the *decode_data* keyword
   with an appropriate value to avoid the deprecation warning.
 
+* :class:`~http.cookies.Morsel` has previously allowed for setting attributes
+  :attr:`~http.cookies.Morsel.key`, :attr:`~http.cookies.Morsel.value` and
+  :attr:`~http.cookies.Morsel.coded_value`. Use the preferred
+  :func:`~http.cookies.Morsel.set` method in order to avoid the deprecation
+  warning.
+
 
 Deprecated functions and types of the C API
 -------------------------------------------
diff --git a/Lib/http/cookies.py b/Lib/http/cookies.py
index 73acbc7..f4e9035 100644
--- a/Lib/http/cookies.py
+++ b/Lib/http/cookies.py
@@ -138,6 +138,12 @@ _nulljoin = ''.join
 _semispacejoin = '; '.join
 _spacejoin = ' '.join
 
+def _warn_deprecated_setter(setter):
+    import warnings
+    msg = ('The .%s setter is deprecated. The attribute will be read-only in '
+           'future releases. Please use the set() method instead.' % setter)
+    warnings.warn(msg, DeprecationWarning, stacklevel=3)
+
 #
 # Define an exception visible to External modules
 #
@@ -151,88 +157,36 @@ class CookieError(Exception):
 # into a 4 character sequence: a forward-slash followed by the
 # three-digit octal equivalent of the character.  Any '\' or '"' is
 # quoted with a preceeding '\' slash.
+# Because of the way browsers really handle cookies (as opposed to what
+# the RFC says) we also encode "," and ";".
 #
 # These are taken from RFC2068 and RFC2109.
 #       _LegalChars       is the list of chars which don't require "'s
 #       _Translator       hash-table for fast quoting
 #
-_LegalChars       = string.ascii_letters + string.digits + "!#$%&'*+-.^_`|~:"
-_Translator       = {
-    '\000' : '\\000',  '\001' : '\\001',  '\002' : '\\002',
-    '\003' : '\\003',  '\004' : '\\004',  '\005' : '\\005',
-    '\006' : '\\006',  '\007' : '\\007',  '\010' : '\\010',
-    '\011' : '\\011',  '\012' : '\\012',  '\013' : '\\013',
-    '\014' : '\\014',  '\015' : '\\015',  '\016' : '\\016',
-    '\017' : '\\017',  '\020' : '\\020',  '\021' : '\\021',
-    '\022' : '\\022',  '\023' : '\\023',  '\024' : '\\024',
-    '\025' : '\\025',  '\026' : '\\026',  '\027' : '\\027',
-    '\030' : '\\030',  '\031' : '\\031',  '\032' : '\\032',
-    '\033' : '\\033',  '\034' : '\\034',  '\035' : '\\035',
-    '\036' : '\\036',  '\037' : '\\037',
-
-    # Because of the way browsers really handle cookies (as opposed
-    # to what the RFC says) we also encode , and ;
-
-    ',' : '\\054', ';' : '\\073',
-
-    '"' : '\\"',       '\\' : '\\\\',
-
-    '\177' : '\\177',  '\200' : '\\200',  '\201' : '\\201',
-    '\202' : '\\202',  '\203' : '\\203',  '\204' : '\\204',
-    '\205' : '\\205',  '\206' : '\\206',  '\207' : '\\207',
-    '\210' : '\\210',  '\211' : '\\211',  '\212' : '\\212',
-    '\213' : '\\213',  '\214' : '\\214',  '\215' : '\\215',
-    '\216' : '\\216',  '\217' : '\\217',  '\220' : '\\220',
-    '\221' : '\\221',  '\222' : '\\222',  '\223' : '\\223',
-    '\224' : '\\224',  '\225' : '\\225',  '\226' : '\\226',
-    '\227' : '\\227',  '\230' : '\\230',  '\231' : '\\231',
-    '\232' : '\\232',  '\233' : '\\233',  '\234' : '\\234',
-    '\235' : '\\235',  '\236' : '\\236',  '\237' : '\\237',
-    '\240' : '\\240',  '\241' : '\\241',  '\242' : '\\242',
-    '\243' : '\\243',  '\244' : '\\244',  '\245' : '\\245',
-    '\246' : '\\246',  '\247' : '\\247',  '\250' : '\\250',
-    '\251' : '\\251',  '\252' : '\\252',  '\253' : '\\253',
-    '\254' : '\\254',  '\255' : '\\255',  '\256' : '\\256',
-    '\257' : '\\257',  '\260' : '\\260',  '\261' : '\\261',
-    '\262' : '\\262',  '\263' : '\\263',  '\264' : '\\264',
-    '\265' : '\\265',  '\266' : '\\266',  '\267' : '\\267',
-    '\270' : '\\270',  '\271' : '\\271',  '\272' : '\\272',
-    '\273' : '\\273',  '\274' : '\\274',  '\275' : '\\275',
-    '\276' : '\\276',  '\277' : '\\277',  '\300' : '\\300',
-    '\301' : '\\301',  '\302' : '\\302',  '\303' : '\\303',
-    '\304' : '\\304',  '\305' : '\\305',  '\306' : '\\306',
-    '\307' : '\\307',  '\310' : '\\310',  '\311' : '\\311',
-    '\312' : '\\312',  '\313' : '\\313',  '\314' : '\\314',
-    '\315' : '\\315',  '\316' : '\\316',  '\317' : '\\317',
-    '\320' : '\\320',  '\321' : '\\321',  '\322' : '\\322',
-    '\323' : '\\323',  '\324' : '\\324',  '\325' : '\\325',
-    '\326' : '\\326',  '\327' : '\\327',  '\330' : '\\330',
-    '\331' : '\\331',  '\332' : '\\332',  '\333' : '\\333',
-    '\334' : '\\334',  '\335' : '\\335',  '\336' : '\\336',
-    '\337' : '\\337',  '\340' : '\\340',  '\341' : '\\341',
-    '\342' : '\\342',  '\343' : '\\343',  '\344' : '\\344',
-    '\345' : '\\345',  '\346' : '\\346',  '\347' : '\\347',
-    '\350' : '\\350',  '\351' : '\\351',  '\352' : '\\352',
-    '\353' : '\\353',  '\354' : '\\354',  '\355' : '\\355',
-    '\356' : '\\356',  '\357' : '\\357',  '\360' : '\\360',
-    '\361' : '\\361',  '\362' : '\\362',  '\363' : '\\363',
-    '\364' : '\\364',  '\365' : '\\365',  '\366' : '\\366',
-    '\367' : '\\367',  '\370' : '\\370',  '\371' : '\\371',
-    '\372' : '\\372',  '\373' : '\\373',  '\374' : '\\374',
-    '\375' : '\\375',  '\376' : '\\376',  '\377' : '\\377'
-    }
+_LegalChars = string.ascii_letters + string.digits + "!#$%&'*+-.^_`|~:"
+_UnescapedChars = _LegalChars + ' ()/<=>?@[]{}'
+
+_Translator = {n: '\\%03o' % n
+               for n in set(range(256)) - set(map(ord, _UnescapedChars))}
+_Translator.update({
+    ord('"'): '\\"',
+    ord('\\'): '\\\\',
+})
 
-def _quote(str, LegalChars=_LegalChars):
+_is_legal_key = re.compile('[%s]+' % _LegalChars).fullmatch
+
+def _quote(str):
     r"""Quote a string for use in a cookie header.
 
     If the string does not need to be double-quoted, then just return the
     string.  Otherwise, surround the string in doublequotes and quote
     (with a \) special characters.
     """
-    if all(c in LegalChars for c in str):
+    if str is None or _is_legal_key(str):
         return str
     else:
-        return '"' + _nulljoin(_Translator.get(s, s) for s in str) + '"'
+        return '"' + str.translate(_Translator) + '"'
 
 
 _OctalPatt = re.compile(r"\\[0-3][0-7][0-7]")
@@ -241,7 +195,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 str is None or len(str) < 2:
         return str
     if str[0] != '"' or str[-1] != '"':
         return str
@@ -339,33 +293,89 @@ class Morsel(dict):
 
     def __init__(self):
         # Set defaults
-        self.key = self.value = self.coded_value = None
+        self._key = self._value = self._coded_value = None
 
         # Set default attributes
         for key in self._reserved:
             dict.__setitem__(self, key, "")
 
+    @property
+    def key(self):
+        return self._key
+
+    @key.setter
+    def key(self, key):
+        _warn_deprecated_setter('key')
+        self._key = key
+
+    @property
+    def value(self):
+        return self._value
+
+    @value.setter
+    def value(self, value):
+        _warn_deprecated_setter('value')
+        self._value = value
+
+    @property
+    def coded_value(self):
+        return self._coded_value
+
+    @coded_value.setter
+    def coded_value(self, coded_value):
+        _warn_deprecated_setter('coded_value')
+        self._coded_value = coded_value
+
     def __setitem__(self, K, V):
         K = K.lower()
         if not K in self._reserved:
-            raise CookieError("Invalid Attribute %s" % K)
+            raise CookieError("Invalid attribute %r" % (K,))
         dict.__setitem__(self, K, V)
 
+    def setdefault(self, key, val=None):
+        key = key.lower()
+        if key not in self._reserved:
+            raise CookieError("Invalid attribute %r" % (key,))
+        return dict.setdefault(self, key, val)
+
+    def __eq__(self, morsel):
+        if not isinstance(morsel, Morsel):
+            return NotImplemented
+        return (dict.__eq__(self, morsel) and
+                self._value == morsel._value and
+                self._key == morsel._key and
+                self._coded_value == morsel._coded_value)
+
+    __ne__ = object.__ne__
+
+    def copy(self):
+        morsel = Morsel()
+        dict.update(morsel, self)
+        morsel.__dict__.update(self.__dict__)
+        return morsel
+
+    def update(self, values):
+        data = {}
+        for key, val in dict(values).items():
+            key = key.lower()
+            if key not in self._reserved:
+                raise CookieError("Invalid attribute %r" % (key,))
+            data[key] = val
+        dict.update(self, data)
+
     def isReservedKey(self, K):
         return K.lower() in self._reserved
 
-    def set(self, key, val, coded_val, LegalChars=_LegalChars):
-        # First we verify that the key isn't a reserved word
-        # Second we make sure it only contains legal characters
+    def set(self, key, val, coded_val):
         if key.lower() in self._reserved:
-            raise CookieError("Attempt to set a reserved key: %s" % key)
-        if any(c not in LegalChars for c in key):
-            raise CookieError("Illegal key value: %s" % key)
+            raise CookieError('Attempt to set a reserved key %r' % (key,))
+        if not _is_legal_key(key):
+            raise CookieError('Illegal key %r' % (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))
@@ -373,8 +383,7 @@ class Morsel(dict):
     __str__ = output
 
     def __repr__(self):
-        return '<%s: %s=%s>' % (self.__class__.__name__,
-                                self.key, repr(self.value))
+        return '<%s: %s>' % (self.__class__.__name__, self.OutputString())
 
     def js_output(self, attrs=None):
         # Print javascript
@@ -408,10 +417,9 @@ class Morsel(dict):
                 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]))
+            elif key in self._flags:
+                if value:
+                    append(str(self._reserved[key]))
             else:
                 append("%s=%s" % (self._reserved[key], value))
 
diff --git a/Lib/test/test_http_cookies.py b/Lib/test/test_http_cookies.py
index ee30f17..7665b15 100644
--- a/Lib/test/test_http_cookies.py
+++ b/Lib/test/test_http_cookies.py
@@ -200,6 +200,15 @@ class CookieTests(unittest.TestCase):
 class MorselTests(unittest.TestCase):
     """Tests for the Morsel object."""
 
+    def test_defaults(self):
+        morsel = cookies.Morsel()
+        self.assertIsNone(morsel.key)
+        self.assertIsNone(morsel.value)
+        self.assertIsNone(morsel.coded_value)
+        self.assertEqual(morsel.keys(), cookies.Morsel._reserved.keys())
+        for key, val in morsel.items():
+            self.assertEqual(val, '', key)
+
     def test_reserved_keys(self):
         M = cookies.Morsel()
         # tests valid and invalid reserved keys for Morsels
@@ -243,6 +252,176 @@ class MorselTests(unittest.TestCase):
             self.assertRaises(cookies.CookieError,
                               M.set, i, '%s_value' % i, '%s_value' % i)
 
+    def test_deprecation(self):
+        morsel = cookies.Morsel()
+        with self.assertWarnsRegex(DeprecationWarning, r'\bkey\b'):
+            morsel.key = ''
+        with self.assertWarnsRegex(DeprecationWarning, r'\bvalue\b'):
+            morsel.value = ''
+        with self.assertWarnsRegex(DeprecationWarning, r'\bcoded_value\b'):
+            morsel.coded_value = ''
+
+    def test_eq(self):
+        base_case = ('key', 'value', '"value"')
+        attribs = {
+            'path': '/',
+            'comment': 'foo',
+            'domain': 'example.com',
+            'version': 2,
+        }
+        morsel_a = cookies.Morsel()
+        morsel_a.update(attribs)
+        morsel_a.set(*base_case)
+        morsel_b = cookies.Morsel()
+        morsel_b.update(attribs)
+        morsel_b.set(*base_case)
+        self.assertTrue(morsel_a == morsel_b)
+        self.assertFalse(morsel_a != morsel_b)
+        cases = (
+            ('key', 'value', 'mismatch'),
+            ('key', 'mismatch', '"value"'),
+            ('mismatch', 'value', '"value"'),
+        )
+        for case_b in cases:
+            with self.subTest(case_b):
+                morsel_b = cookies.Morsel()
+                morsel_b.update(attribs)
+                morsel_b.set(*case_b)
+                self.assertFalse(morsel_a == morsel_b)
+                self.assertTrue(morsel_a != morsel_b)
+
+        morsel_b = cookies.Morsel()
+        morsel_b.update(attribs)
+        morsel_b.set(*base_case)
+        morsel_b['comment'] = 'bar'
+        self.assertFalse(morsel_a == morsel_b)
+        self.assertTrue(morsel_a != morsel_b)
+
+        # test mismatched types
+        self.assertFalse(cookies.Morsel() == 1)
+        self.assertTrue(cookies.Morsel() != 1)
+        self.assertFalse(cookies.Morsel() == '')
+        self.assertTrue(cookies.Morsel() != '')
+        items = list(cookies.Morsel().items())
+        self.assertFalse(cookies.Morsel() == items)
+        self.assertTrue(cookies.Morsel() != items)
+
+        # morsel/dict
+        morsel = cookies.Morsel()
+        morsel.set(*base_case)
+        morsel.update(attribs)
+        self.assertTrue(morsel == dict(morsel))
+        self.assertFalse(morsel != dict(morsel))
+
+    def test_copy(self):
+        morsel_a = cookies.Morsel()
+        morsel_a.set('foo', 'bar', 'baz')
+        morsel_a.update({
+            'version': 2,
+            'comment': 'foo',
+        })
+        morsel_b = morsel_a.copy()
+        self.assertIsInstance(morsel_b, cookies.Morsel)
+        self.assertIsNot(morsel_a, morsel_b)
+        self.assertEqual(morsel_a, morsel_b)
+
+    def test_setitem(self):
+        morsel = cookies.Morsel()
+        morsel['expires'] = 0
+        self.assertEqual(morsel['expires'], 0)
+        morsel['Version'] = 2
+        self.assertEqual(morsel['version'], 2)
+        morsel['DOMAIN'] = 'example.com'
+        self.assertEqual(morsel['domain'], 'example.com')
+
+        with self.assertRaises(cookies.CookieError):
+            morsel['invalid'] = 'value'
+        self.assertNotIn('invalid', morsel)
+
+    def test_setdefault(self):
+        morsel = cookies.Morsel()
+        morsel.update({
+            'domain': 'example.com',
+            'version': 2,
+        })
+        # this shouldn't override the default value
+        self.assertEqual(morsel.setdefault('expires', 'value'), '')
+        self.assertEqual(morsel['expires'], '')
+        self.assertEqual(morsel.setdefault('Version', 1), 2)
+        self.assertEqual(morsel['version'], 2)
+        self.assertEqual(morsel.setdefault('DOMAIN', 'value'), 'example.com')
+        self.assertEqual(morsel['domain'], 'example.com')
+
+        with self.assertRaises(cookies.CookieError):
+            morsel.setdefault('invalid', 'value')
+        self.assertNotIn('invalid', morsel)
+
+    def test_update(self):
+        attribs = {'expires': 1, 'Version': 2, 'DOMAIN': 'example.com'}
+        # test dict update
+        morsel = cookies.Morsel()
+        morsel.update(attribs)
+        self.assertEqual(morsel['expires'], 1)
+        self.assertEqual(morsel['version'], 2)
+        self.assertEqual(morsel['domain'], 'example.com')
+        # test iterable update
+        morsel = cookies.Morsel()
+        morsel.update(list(attribs.items()))
+        self.assertEqual(morsel['expires'], 1)
+        self.assertEqual(morsel['version'], 2)
+        self.assertEqual(morsel['domain'], 'example.com')
+        # test iterator update
+        morsel = cookies.Morsel()
+        morsel.update((k, v) for k, v in attribs.items())
+        self.assertEqual(morsel['expires'], 1)
+        self.assertEqual(morsel['version'], 2)
+        self.assertEqual(morsel['domain'], 'example.com')
+
+        with self.assertRaises(cookies.CookieError):
+            morsel.update({'invalid': 'value'})
+        self.assertNotIn('invalid', morsel)
+        self.assertRaises(TypeError, morsel.update)
+        self.assertRaises(TypeError, morsel.update, 0)
+
+    def test_repr(self):
+        morsel = cookies.Morsel()
+        self.assertEqual(repr(morsel), '<Morsel: None=None>')
+        self.assertEqual(str(morsel), 'Set-Cookie: None=None')
+        morsel.set('key', 'val', 'coded_val')
+        self.assertEqual(repr(morsel), '<Morsel: key=coded_val>')
+        self.assertEqual(str(morsel), 'Set-Cookie: key=coded_val')
+        morsel.update({
+            'path': '/',
+            'comment': 'foo',
+            'domain': 'example.com',
+            'max-age': 0,
+            'secure': 0,
+            'version': 1,
+        })
+        self.assertEqual(repr(morsel),
+                '<Morsel: key=coded_val; Comment=foo; Domain=example.com; '
+                'Max-Age=0; Path=/; Version=1>')
+        self.assertEqual(str(morsel),
+                'Set-Cookie: key=coded_val; Comment=foo; Domain=example.com; '
+                'Max-Age=0; Path=/; Version=1')
+        morsel['secure'] = True
+        morsel['httponly'] = 1
+        self.assertEqual(repr(morsel),
+                '<Morsel: key=coded_val; Comment=foo; Domain=example.com; '
+                'HttpOnly; Max-Age=0; Path=/; Secure; Version=1>')
+        self.assertEqual(str(morsel),
+                'Set-Cookie: key=coded_val; Comment=foo; Domain=example.com; '
+                'HttpOnly; Max-Age=0; Path=/; Secure; Version=1')
+
+        morsel = cookies.Morsel()
+        morsel.set('key', 'val', 'coded_val')
+        morsel['expires'] = 0
+        self.assertRegex(repr(morsel),
+                r'<Morsel: key=coded_val; '
+                r'expires=\w+, \d+ \w+ \d+ \d+:\d+:\d+ \w+>')
+        self.assertRegex(str(morsel),
+                r'Set-Cookie: key=coded_val; '
+                r'expires=\w+, \d+ \w+ \d+ \d+:\d+:\d+ \w+')
 
 def test_main():
     run_unittest(CookieTests, MorselTests)
diff --git a/Misc/NEWS b/Misc/NEWS
index 328b8d5..6f79a73 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -18,6 +18,14 @@ Core and Builtins
 Library
 -------
 
+- Issue #2211: Updated the implementation of the http.cookies.Morsel class.
+  Setting attributes key, value and coded_value directly now is deprecated.
+  update() and setdefault() now transform and check keys.  Comparing for
+  equality now takes into account attributes key, value and coded_value.
+  copy() now returns a Morsel, not a dict.  repr() now contains all attributes.
+  Optimized checking keys and quoting values.  Added new tests.
+  Original patch by Demian Brecht.
+
 - Issue #18983: Allow selection of output units in timeit.
   Patch by Julian Gindi.