qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kp...@apache.org
Subject [qpid-proton] 01/01: PROTON-2237: Correct checking of Proton message property keys
Date Mon, 07 Dec 2020 18:13:47 GMT
This is an automated email from the ASF dual-hosted git repository.

kpvdr pushed a commit to branch python-check-property-keys
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git

commit 66f8696721fab2cc982c05eb7d94572bb2956841
Author: Kim van der Riet <kpvdr@apache.org>
AuthorDate: Tue Jun 2 13:16:49 2020 -0400

    PROTON-2237: Correct checking of Proton message property keys
    
    PROTON-2237: Alternative approach which converts all child classes of string/unicode to
the base class, including proton symbol and char types.
    
    PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol
and proton.char will be encoded as strings
    
    PROTON-2237: Added unit tests to check illegal key types are detected and handled, also
subclasses of string type keys are converted to type string
    
    PROTON-2237: Finalized handling property keys, added tests for these cases.
    
    PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key
conversions
    
    PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what
is happening.
    
    NO-JIRA: Minor code format fix: added space into if stmt.
    
    PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of
tests.
---
 python/proton/_message.py            | 41 +++++++++++++++----
 python/tests/proton_tests/message.py | 78 ++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+), 8 deletions(-)

diff --git a/python/proton/_message.py b/python/proton/_message.py
index 6563935..ed791c9 100644
--- a/python/proton/_message.py
+++ b/python/proton/_message.py
@@ -34,7 +34,7 @@ from cproton import PN_DEFAULT_PRIORITY, PN_OVERFLOW, pn_error_text, pn_message,
 
 from . import _compat
 from ._common import isinteger, millis2secs, secs2millis, unicode2utf8, utf82unicode
-from ._data import Data, symbol, ulong, AnnotationDict
+from ._data import char, Data, decimal128, symbol, ulong, AnnotationDict
 from ._endpoints import Link
 from ._exceptions import EXCEPTIONS, MessageException
 
@@ -89,16 +89,41 @@ class Message(object):
             return err
 
     def _check_property_keys(self):
+        '''
+        AMQP allows only string keys for properties. This function checks that this requirement
is met
+        and raises a MessageException if not. However, in certain cases, conversions to string
are
+        automatically performed:
+
+        1. When a key is a user-defined (non-AMQP) subclass of unicode/str (py2/py3).
+           AMQP types symbol and char, although derived from unicode/str, are not converted,
+           and result in an exception.
+
+        2. In Py2, when a key is binary. This is a convenience for Py2 users that encode
+           string literals without using the u'' prefix. In Py3, this is not the case, and
+           using a binary key will result in an error. AMQP type decimal128, which is derived
+           from binary, will not be converted, and will result in an exception.
+        '''
+        # We cannot make changes to the dict while iterating, so we
+        # must save and make the changes afterwards
+        changed_keys = []
         for k in self.properties.keys():
             if isinstance(k, unicode):
-                # py2 unicode, py3 str (via hack definition)
-                continue
-            # If key is binary then change to string
-            elif isinstance(k, str):
-                # py2 str
-                self.properties[k.encode('utf-8')] = self.properties.pop(k)
+                # Py2 & Py3 strings and their subclasses
+                if type(k) is symbol or type(k) is char:
+                    # Exclude symbol and char
+                    raise MessageException('Application property key is not string type:
key=%s %s' % (str(k), type(k)))
+                if type(k) is not unicode:
+                    # Only for string subclasses, convert to string
+                    changed_keys.append((k, unicode(k)))
+            elif isinstance(k, str) and not (type(k) is decimal128):
+                # Py2 only: If key is binary string then convert to unicode. Exclude bytes
subclass decimal128.
+                changed_keys.append((k, k.decode('utf-8')))
             else:
+                # Anything else: raise exception
                 raise MessageException('Application property key is not string type: key=%s
%s' % (str(k), type(k)))
+        # Make the key changes
+        for old_key, new_key in changed_keys:
+            self.properties[new_key] = self.properties.pop(old_key)
 
     def _pre_encode(self):
         inst = Data(pn_message_instructions(self._msg))
@@ -427,7 +452,7 @@ class Message(object):
         The group-id for any replies.
 
         :type: ``str``
-        :raise: :exc:`MessageException` if there is any Proton error when using the setter.
       
+        :raise: :exc:`MessageException` if there is any Proton error when using the setter.
         """)
 
     def _get_instructions(self):
diff --git a/python/tests/proton_tests/message.py b/python/tests/proton_tests/message.py
index 04413ee..6f407ca 100644
--- a/python/tests/proton_tests/message.py
+++ b/python/tests/proton_tests/message.py
@@ -20,6 +20,8 @@
 from __future__ import absolute_import
 
 from uuid import uuid4
+from sys import version_info
+from unittest import skipIf
 
 from proton import *
 
@@ -107,6 +109,15 @@ class AccessorsTest(Test):
   def testReplyToGroupId(self):
     self._test_str("reply_to_group_id")
 
+try:
+    long()
+except NameError:
+    long = int
+try:
+    unicode()
+except NameError:
+    unicode = str
+
 class CodecTest(Test):
 
   def testProperties(self):
@@ -119,6 +130,73 @@ class CodecTest(Test):
 
     assert msg2.properties['key'] == 'value', msg2.properties['key']
 
+  class MyStringSubclass(unicode):
+    def __repr__(self):
+      return 'MyStringSubclass(%s)' % unicode.__repr__(self)
+
+  def testStringSubclassPropertyKey(self):
+    self.msg.properties = {'abc': 123, CodecTest.MyStringSubclass('def'): 456}
+    data = self.msg.encode()
+
+    msg2 = Message()
+    msg2.decode(data)
+
+    assert msg2.properties == self.msg.properties
+    for k in msg2.properties:
+        assert type(k) is unicode, 'non-string key %s %s' % (k, type(k))
+
+  def _testNonStringPropertyKey(self, k):
+    self.msg.properties = {k: 'abc'}
+    try:
+        self.msg.encode()
+    except MessageException:
+        return
+    self.fail('Non-string property key: %s, key type: %s' % (self.msg, type(k)))
+
+  def testNonSequencePropertyKeys(self):
+    # AMQP non-string types
+    self._testNonStringPropertyKey(None)
+    self._testNonStringPropertyKey(True)
+    self._testNonStringPropertyKey(byte(1))
+    self._testNonStringPropertyKey(ubyte(2))
+    self._testNonStringPropertyKey(short(3))
+    self._testNonStringPropertyKey(ushort(4))
+    self._testNonStringPropertyKey(int32(5))
+    self._testNonStringPropertyKey(uint(6))
+    self._testNonStringPropertyKey(long(7))
+    self._testNonStringPropertyKey(ulong(8))
+    self._testNonStringPropertyKey(float32(9.0))
+    self._testNonStringPropertyKey(10.0)
+    self._testNonStringPropertyKey(decimal32(11))
+    self._testNonStringPropertyKey(decimal64(12))
+    self._testNonStringPropertyKey(timestamp(1234567890))
+    self._testNonStringPropertyKey(uuid4())
+
+  def testCharPropertyKey(self):
+    self._testNonStringPropertyKey(char('A'))
+
+  def testDecimal128PropertyKey(self):
+    self._testNonStringPropertyKey(decimal128(b'12345'))
+
+  def testSymbolPropertyKey(self):
+    self._testNonStringPropertyKey(symbol('abcdef'))
+
+  def testBinaryPropertyKey(self):
+    if version_info[0] > 2:
+      # Py3: Binary conversions not supported
+      self._testNonStringPropertyKey(b'abc123')
+    else:
+      # Py2: Check for correct key conversion
+      self.msg.properties = {'abc': 123, b'def': 456}
+      data = self.msg.encode()
+
+      msg2 = Message()
+      msg2.decode(data)
+
+      assert msg2.properties == self.msg.properties
+      for k in msg2.properties:
+        assert type(k) is unicode, 'non-string key %s %s' % (k, type(k))
+
   def testAnnotationsSymbolicAndUlongKey(self, a={symbol('one'): 1, 'two': 2, ulong(3): 'three'}):
     self.msg.annotations = a
     data = self.msg.encode()


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


Mime
View raw message