asText survives parseFilter output: to_unicode instead of .decode() (fixes #248)

docs/source/NEWS.rst

@@ -87,6 +87,12 @@ Fixes
   which matters for TLS teardown. A new
   ``LDAPClient.notifyOnDisconnect()`` helper lets callers register
   additional disconnect Deferreds without initiating an unbind (#225).
+- ``asText`` on filters produced by ``parseFilter`` (whose ``.value``
+  fields are ``str`` rather than the wire-decoded ``bytes``) no longer
+  raises ``AttributeError: 'str' object has no attribute 'decode'``.
+  All ten remaining ``.value.decode()`` sites in ``pureldap.py`` now go
+  through ``to_unicode`` so both str-constructed and wire-decoded
+  filters render (#248, follow-up to #226).
 
 
 21.2.0 (2021-02-28)

ldaptor/protocols/pureldap.py

@@ -20,7 +20,7 @@
     berDecodeObject,
     int2berlen,
 )
-from ldaptor._encoder import to_bytes
+from ldaptor._encoder import to_bytes, to_unicode
 
 next_ldap_message_id = 1
 
@@ -571,9 +571,9 @@ class LDAPFilter_equalityMatch(LDAPAttributeValueAssertion):
     def asText(self):
         return (
             "("
-            + self.attributeDesc.value.decode()
+            + to_unicode(self.attributeDesc.value)
             + "="
-            + self.escaper(self.assertionValue.value.decode())
+            + self.escaper(to_unicode(self.assertionValue.value))
             + ")"
         )
 
@@ -582,21 +582,21 @@ class LDAPFilter_substrings_initial(LDAPString):
     tag = CLASS_CONTEXT | 0x00
 
     def asText(self):
-        return self.escaper(self.value.decode())
+        return self.escaper(to_unicode(self.value))
 
 
 class LDAPFilter_substrings_any(LDAPString):
     tag = CLASS_CONTEXT | 0x01
 
     def asText(self):
-        return self.escaper(self.value.decode())
+        return self.escaper(to_unicode(self.value))
 
 
 class LDAPFilter_substrings_final(LDAPString):
     tag = CLASS_CONTEXT | 0x02
 
     def asText(self):
-        return self.escaper(self.value.decode())
+        return self.escaper(to_unicode(self.value))
 
 
 class LDAPBERDecoderContext_Filter_substrings(BERDecoderContext):
@@ -674,7 +674,11 @@ def asText(self):
             final = ""
 
         return (
-            "(" + self.type.decode() + "=" + "*".join([initial] + any + [final]) + ")"
+            "("
+            + to_unicode(self.type)
+            + "="
+            + "*".join([initial] + any + [final])
+            + ")"
         )
 
 
@@ -684,9 +688,9 @@ class LDAPFilter_greaterOrEqual(LDAPAttributeValueAssertion):
     def asText(self):
         return (
             "("
-            + self.attributeDesc.value.decode()
+            + to_unicode(self.attributeDesc.value)
             + ">="
-            + self.escaper(self.assertionValue.value.decode())
+            + self.escaper(to_unicode(self.assertionValue.value))
             + ")"
         )
 
@@ -697,9 +701,9 @@ class LDAPFilter_lessOrEqual(LDAPAttributeValueAssertion):
     def asText(self):
         return (
             "("
-            + self.attributeDesc.value.decode()
+            + to_unicode(self.attributeDesc.value)
             + "<="
-            + self.escaper(self.assertionValue.value.decode())
+            + self.escaper(to_unicode(self.assertionValue.value))
             + ")"
         )
 
@@ -708,7 +712,7 @@ class LDAPFilter_present(LDAPAttributeDescription):
     tag = CLASS_CONTEXT | 0x07
 
     def asText(self):
-        return "(" + self.value.decode() + "=*)"
+        return "(" + to_unicode(self.value) + "=*)"
 
 
 class LDAPFilter_approxMatch(LDAPAttributeValueAssertion):
@@ -717,9 +721,9 @@ class LDAPFilter_approxMatch(LDAPAttributeValueAssertion):
     def asText(self):
         return (
             "("
-            + self.attributeDesc.value.decode()
+            + to_unicode(self.attributeDesc.value)
             + "~="
-            + self.escaper(self.assertionValue.value.decode())
+            + self.escaper(to_unicode(self.assertionValue.value))
             + ")"
         )
 
@@ -858,11 +862,11 @@ class LDAPFilter_extensibleMatch(LDAPMatchingRuleAssertion):
     def asText(self):
         return (
             "("
-            + (self.type.value.decode() if self.type else "")
+            + (to_unicode(self.type.value) if self.type else "")
             + (":dn" if self.dnAttributes and self.dnAttributes.value else "")
-            + ((":" + self.matchingRule.value.decode()) if self.matchingRule else "")
+            + ((":" + to_unicode(self.matchingRule.value)) if self.matchingRule else "")
             + ":="
-            + self.escaper(self.matchValue.value.decode())
+            + self.escaper(to_unicode(self.matchValue.value))
             + ")"
         )
 

ldaptor/test/test_ldapfilter.py

@@ -646,3 +646,34 @@ def test_escape(self):
         self.assertRaises(
             ldapfilter.InvalidLDAPFilter, ldapfilter.parseFilter, r"(cn=\ 61)"
         )
+
+
+class TestParseFilterAsTextRoundtrip(unittest.TestCase):
+    """
+    ``asText`` must work on filters produced by ``parseFilter`` (whose
+    ``value`` fields are ``str``) as well as on wire-decoded filters
+    (whose ``value`` fields are ``bytes``). Previously several
+    ``asText`` implementations called ``.decode()`` unconditionally and
+    raised ``AttributeError: 'str' object has no attribute 'decode'``
+    when handed a parsed filter (#248).
+    """
+
+    cases = [
+        "(cn=foo)",
+        "(cn=*)",
+        "(cn=foo*)",
+        "(cn=*foo)",
+        "(cn=foo*bar*baz)",
+        "(cn~=foo)",
+        "(cn>=1)",
+        "(cn<=9)",
+        "(cn:caseIgnoreMatch:=foo)",
+    ]
+
+    def test_asText_survives_parseFilter(self):
+        for text in self.cases:
+            filt = ldapfilter.parseFilter(text)
+            # asText must not raise, and must return a str.
+            self.assertIsInstance(
+                filt.asText(), str, f"asText on {text!r} did not return str"
+            )