diff --git a/docs/source/NEWS.rst b/docs/source/NEWS.rst index 7e826c55..4f51cd31 100644 --- a/docs/source/NEWS.rst +++ b/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) diff --git a/ldaptor/protocols/pureldap.py b/ldaptor/protocols/pureldap.py index 0528d99a..3af0bfdc 100644 --- a/ldaptor/protocols/pureldap.py +++ b/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)) + ")" ) diff --git a/ldaptor/test/test_ldapfilter.py b/ldaptor/test/test_ldapfilter.py index a7d392e8..1fb96157 100644 --- a/ldaptor/test/test_ldapfilter.py +++ b/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" + )