Update rule comparison operators to work with mixed operator types (bytes and strings)

CHANGELOG.rst

@@ -88,6 +88,9 @@ Fixed
 * Fixed ``core.sendmail`` base64 encoding of longer subject lines (bug fix) #4795
 
   Contributed by @stevemuskiewicz and @guzzijones
+* Update all the various rule criteria comparison operators which also work with strings (equals,
+  icontains, nequals, etc.) to work correctly on Python 3 deployments if one of the operators is
+  of a type bytes and the other is of a type unicode / string. (bug fix) #4831
 
 3.1.0 - June 27, 2019
 ---------------------
@@ -131,7 +134,7 @@ Fixed
   value for SSH port is specified in the configured SSH config file
   (``ssh_runner.ssh_config_file_path``). (bug fix) #4660 #4661
 * Update pack install action so it works correctly when ``python_versions`` ``pack.yaml`` metadata
-  attribute is used in combination with ``--python3`` pack install flag. (bug fix) #4654 #4662
+  attribute is used in combination with ``--use-python3`` pack install flag. (bug fix) #4654 #4662
 * Add ``source_channel`` back to the context used by Mistral workflows for executions which are
   triggered via ChatOps (using action alias).
 
@@ -141,7 +144,7 @@ Fixed
   server time where st2api is running was not set to UTC. (bug fix) #4668
 
   Contributed by Igor Cherkaev. (@emptywee)
-* Fix a bug with some packs which use ``--python3`` flag (running Python 3 actions on installation
+* Fix a bug with some packs which use ``--use-python3`` flag (running Python 3 actions on installation
   where StackStorm components run under Python 2) which rely on modules from Python 3 standard
   library which are also available in Python 2 site-packages (e.g. ``concurrent``) not working
   correctly.

contrib/hello_st2/pack.yaml

@@ -1,5 +1,6 @@
 ---
-# Pack reference. It can only contain letters, digits and underscores.
+# Pack reference. It can only contain lowercase letters, digits and underscores.
+# This attribute is only needed if "name" attribute contains special characters.
 ref: hello_st2
 # User-friendly pack name. If this attribute contains spaces or any other special characters, then
 # the "ref" attribute must also be specified (see above).

st2common/st2common/operators.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 from __future__ import absolute_import
+
 import re
 import six
 import fnmatch
@@ -140,64 +141,85 @@ def search(value, criteria_pattern, criteria_condition, check_function):
 def equals(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value == criteria_pattern
 
 
 def nequals(value, criteria_pattern):
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value != criteria_pattern
 
 
 def iequals(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value.lower() == criteria_pattern.lower()
 
 
 def contains(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return criteria_pattern in value
 
 
 def icontains(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return criteria_pattern.lower() in value.lower()
 
 
 def ncontains(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return criteria_pattern not in value
 
 
 def incontains(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return criteria_pattern.lower() not in value.lower()
 
 
 def startswith(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value.startswith(criteria_pattern)
 
 
 def istartswith(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value.lower().startswith(criteria_pattern.lower())
 
 
 def endswith(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value.endswith(criteria_pattern)
 
 
 def iendswith(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value.lower().endswith(criteria_pattern.lower())
 
 
@@ -217,13 +239,16 @@ def match_wildcard(value, criteria_pattern):
     if criteria_pattern is None:
         return False
 
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return fnmatch.fnmatch(value, criteria_pattern)
 
 
 def match_regex(value, criteria_pattern):
     # match_regex is deprecated, please use 'regex' and 'iregex'
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     regex = re.compile(criteria_pattern, re.DOTALL)
     # check for a match and not for details of the match.
     return regex.match(value) is not None
@@ -232,6 +257,8 @@ def match_regex(value, criteria_pattern):
 def regex(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     regex = re.compile(criteria_pattern)
     # check for a match and not for details of the match.
     return regex.search(value) is not None
@@ -240,6 +267,8 @@ def regex(value, criteria_pattern):
 def iregex(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     regex = re.compile(criteria_pattern, re.IGNORECASE)
     # check for a match and not for details of the match.
     return regex.search(value) is not None
@@ -288,15 +317,40 @@ def nexists(value, criteria_pattern):
 def inside(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value in criteria_pattern
 
 
 def ninside(value, criteria_pattern):
     if criteria_pattern is None:
         return False
+
+    value, criteria_pattern = ensure_operators_are_strings(value, criteria_pattern)
     return value not in criteria_pattern
 
 
+def ensure_operators_are_strings(value, criteria_pattern):
+    """
+    This function ensures that both value and criteria_pattern arguments are unicode (string)
+    values if the input value type is bytes.
+
+    If a value is of types bytes and not a unicode, it's converted to unicode. This way we
+    ensure all the operators which expect string / unicode values work, even if one of the
+    values is bytes (this can happen when input is not controlled by the end user - e.g. trigger
+    payload under Python 3 deployments).
+
+    :return: tuple(value, criteria_pattern)
+    """
+    if isinstance(value, bytes):
+        value = value.decode('utf-8')
+
+    if isinstance(criteria_pattern, bytes):
+        criteria_pattern = criteria_pattern.decode('utf-8')
+
+    return value, criteria_pattern
+
+
 # operator match strings
 MATCH_WILDCARD = 'matchwildcard'
 MATCH_REGEX = 'matchregex'

st2common/tests/unit/test_operators.py

@@ -499,6 +499,13 @@ def test_matchwildcard(self):
         self.assertTrue(op('bar', 'b*r'), 'Failed matchwildcard.')
         self.assertTrue(op('bar', 'b?r'), 'Failed matchwildcard.')
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'bar', 'b?r'), 'Failed matchwildcard.')
+        self.assertTrue(op('bar', b'b?r'), 'Failed matchwildcard.')
+        self.assertTrue(op(b'bar', b'b?r'), 'Failed matchwildcard.')
+        self.assertTrue(op(u'bar', b'b?r'), 'Failed matchwildcard.')
+        self.assertTrue(op(u'bar', u'b?r'), 'Failed matchwildcard.')
+
         self.assertFalse(op('1', None), 'Passed matchwildcard with None as criteria_pattern.')
 
     def test_matchregex(self):
@@ -517,13 +524,27 @@ def test_matchregex(self):
         string = 'foo\r\nponies\nbar\nfooooo'
         self.assertTrue(op(string, '.*ponies.*'), 'Failed matchregex.')
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'foo ponies bar', '.*ponies.*'), 'Failed matchregex.')
+        self.assertTrue(op('foo ponies bar', b'.*ponies.*'), 'Failed matchregex.')
+        self.assertTrue(op(b'foo ponies bar', b'.*ponies.*'), 'Failed matchregex.')
+        self.assertTrue(op(b'foo ponies bar', u'.*ponies.*'), 'Failed matchregex.')
+        self.assertTrue(op(u'foo ponies bar', u'.*ponies.*'), 'Failed matchregex.')
+
     def test_iregex(self):
         op = operators.get_operator('iregex')
         self.assertTrue(op('V1', 'v1$'), 'Failed iregex.')
 
         string = 'fooPONIESbarfooooo'
         self.assertTrue(op(string, 'ponies'), 'Failed iregex.')
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'fooPONIESbarfooooo', 'ponies'), 'Failed iregex.')
+        self.assertTrue(op('fooPONIESbarfooooo', b'ponies'), 'Failed iregex.')
+        self.assertTrue(op(b'fooPONIESbarfooooo', b'ponies'), 'Failed iregex.')
+        self.assertTrue(op(b'fooPONIESbarfooooo', u'ponies'), 'Failed iregex.')
+        self.assertTrue(op(u'fooPONIESbarfooooo', u'ponies'), 'Failed iregex.')
+
     def test_iregex_fail(self):
         op = operators.get_operator('iregex')
         self.assertFalse(op('V1_foo', 'v1$'), 'Passed iregex.')
@@ -543,6 +564,13 @@ def test_regex(self):
         string = 'apple unicorns oranges'
         self.assertTrue(op(string, '(ponies|unicorns)'), 'Failed regex.')
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'apples unicorns oranges', '(ponies|unicorns)'), 'Failed regex.')
+        self.assertTrue(op('apples unicorns oranges', b'(ponies|unicorns)'), 'Failed regex.')
+        self.assertTrue(op(b'apples unicorns oranges', b'(ponies|unicorns)'), 'Failed regex.')
+        self.assertTrue(op(b'apples unicorns oranges', u'(ponies|unicorns)'), 'Failed regex.')
+        self.assertTrue(op(u'apples unicorns oranges', u'(ponies|unicorns)'), 'Failed regex.')
+
         string = 'apple unicorns oranges'
         self.assertFalse(op(string, '(pikachu|snorlax|charmander)'), 'Passed regex.')
 
@@ -575,6 +603,13 @@ def test_equals_string(self):
         self.assertTrue(op('1', '1'), 'Failed equals.')
         self.assertTrue(op('', ''), 'Failed equals.')
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'1', '1'), 'Failed equals.')
+        self.assertTrue(op('1', b'1'), 'Failed equals.')
+        self.assertTrue(op(b'1', b'1'), 'Failed equals.')
+        self.assertTrue(op(b'1', u'1'), 'Failed equals.')
+        self.assertTrue(op(u'1', u'1'), 'Failed equals.')
+
     def test_equals_fail(self):
         op = operators.get_operator('equals')
         self.assertFalse(op('1', '2'), 'Passed equals.')
@@ -597,6 +632,13 @@ def test_iequals(self):
         self.assertTrue(op('ABC', 'abc'), 'Failed iequals.')
         self.assertTrue(op('AbC', 'aBc'), 'Failed iequals.')
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'AbC', 'aBc'), 'Failed iequals.')
+        self.assertTrue(op('AbC', b'aBc'), 'Failed iequals.')
+        self.assertTrue(op(b'AbC', b'aBc'), 'Failed iequals.')
+        self.assertTrue(op(b'AbC', u'aBc'), 'Failed iequals.')
+        self.assertTrue(op(u'AbC', u'aBc'), 'Failed iequals.')
+
     def test_iequals_fail(self):
         op = operators.get_operator('iequals')
         self.assertFalse(op('ABC', 'BCA'), 'Passed iequals.')
@@ -611,6 +653,13 @@ def test_contains(self):
         self.assertTrue(op('haystackneedle', 'needle'))
         self.assertTrue(op('haystack needle', 'needle'))
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'haystack needle', 'needle'))
+        self.assertTrue(op('haystack needle', b'needle'))
+        self.assertTrue(op(b'haystack needle', b'needle'))
+        self.assertTrue(op(b'haystack needle', u'needle'))
+        self.assertTrue(op(u'haystack needle', b'needle'))
+
     def test_contains_fail(self):
         op = operators.get_operator('contains')
         self.assertFalse(op('hasystack needl haystack', 'needle'))
@@ -626,6 +675,13 @@ def test_icontains(self):
         self.assertTrue(op('haystackNEEDLE', 'needle'))
         self.assertTrue(op('haystack needle', 'NEEDLE'))
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'haystack needle', 'NEEDLE'))
+        self.assertTrue(op('haystack needle', b'NEEDLE'))
+        self.assertTrue(op(b'haystack needle', b'NEEDLE'))
+        self.assertTrue(op(b'haystack needle', u'NEEDLE'))
+        self.assertTrue(op(u'haystack needle', b'NEEDLE'))
+
     def test_icontains_fail(self):
         op = operators.get_operator('icontains')
         self.assertFalse(op('hasystack needl haystack', 'needle'))
@@ -641,6 +697,13 @@ def test_ncontains(self):
         self.assertTrue(op('haystackneedle', 'needlex'))
         self.assertTrue(op('haystack needle', 'needlex'))
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'haystack needle', 'needlex'))
+        self.assertTrue(op('haystack needle', b'needlex'))
+        self.assertTrue(op(b'haystack needle', b'needlex'))
+        self.assertTrue(op(b'haystack needle', u'needlex'))
+        self.assertTrue(op(u'haystack needle', b'needlex'))
+
     def test_ncontains_fail(self):
         op = operators.get_operator('ncontains')
         self.assertFalse(op('hasystack needle haystack', 'needle'))
@@ -667,6 +730,13 @@ def test_startswith(self):
         self.assertTrue(op('hasystack needle haystack', 'hasystack'))
         self.assertTrue(op('a hasystack needle haystack', 'a '))
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'haystack needle', 'haystack'))
+        self.assertTrue(op('haystack needle', b'haystack'))
+        self.assertTrue(op(b'haystack needle', b'haystack'))
+        self.assertTrue(op(b'haystack needle', u'haystack'))
+        self.assertTrue(op(u'haystack needle', b'haystack'))
+
     def test_startswith_fail(self):
         op = operators.get_operator('startswith')
         self.assertFalse(op('hasystack needle haystack', 'needle'))
@@ -678,6 +748,13 @@ def test_istartswith(self):
         self.assertTrue(op('haystack needle haystack', 'HAYstack'))
         self.assertTrue(op('HAYSTACK needle haystack', 'haystack'))
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'HAYSTACK needle haystack', 'haystack'))
+        self.assertTrue(op('HAYSTACK needle haystack', b'haystack'))
+        self.assertTrue(op(b'HAYSTACK needle haystack', b'haystack'))
+        self.assertTrue(op(b'HAYSTACK needle haystack', u'haystack'))
+        self.assertTrue(op(u'HAYSTACK needle haystack', b'haystack'))
+
     def test_istartswith_fail(self):
         op = operators.get_operator('istartswith')
         self.assertFalse(op('hasystack needle haystack', 'NEEDLE'))
@@ -689,6 +766,13 @@ def test_endswith(self):
         self.assertTrue(op('hasystack needle haystackend', 'haystackend'))
         self.assertTrue(op('a hasystack needle haystack b', 'b'))
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'a hasystack needle haystack b', 'b'))
+        self.assertTrue(op('a hasystack needle haystack b', b'b'))
+        self.assertTrue(op(b'a hasystack needle haystack b', b'b'))
+        self.assertTrue(op(b'a hasystack needle haystack b', u'b'))
+        self.assertTrue(op(u'a hasystack needle haystack b', b'b'))
+
     def test_endswith_fail(self):
         op = operators.get_operator('endswith')
         self.assertFalse(op('hasystack needle haystackend', 'haystack'))
@@ -776,6 +860,11 @@ def test_inside(self):
         self.assertFalse(op('a', 'bcd'), 'Should return False')
         self.assertTrue(op('a', 'abc'), 'Should return True')
 
+        # Mixing bytes and strings / unicode should still work
+        self.assertTrue(op(b'a', 'abc'), 'Should return True')
+        self.assertTrue(op('a', b'abc'), 'Should return True')
+        self.assertTrue(op(b'a', b'abc'), 'Should return True')
+
     def test_ninside(self):
         op = operators.get_operator('ninside')
         self.assertFalse(op('a', None), 'Should return False')