pingidentity / ldapsdk

UnboundID LDAP SDK for Java
Other
327 stars 79 forks source link

Filter.createSubstringFilter with "subInitial" or "subFinal" set to an empty string #154

Open daniel-deptula opened 10 months ago

daniel-deptula commented 10 months ago

Hi,

1) Filter filter = Filter.createSubstringFilter(attrName, null, subAny, null); LDAP search with filter created this way works fine.

2) Filter filter = Filter.createSubstringFilter(attrName, "", subAny, ""); Writing this code I expected filter to be the same as in the first example. filter.toString() returns the same string as in the first example but LDAP search fails. For example Active Directory immediately returns 0 results (with resultCode=0) and 389 Directory Server throws resultCode=21 (invalid attribute syntax) diagnosticMessage='Bad search filter'. I haven't tested with other LDAP servers.

In my opinion both versions should return the same valid filter. If I'm wrong and there's a valid reason behind this behavior, it should be clearly explained in the documentation of the Filter class.

dirmgr commented 9 months ago

The LDAP SDK's API for constructing substring filters was created with the assumption that if a subInitial, subAny, or subFinal component wasn't needed, then it wouldn't be provided rather than given as an empty string. This is more in line with how filters are transmitted over the wire (they aren't sent as strings, but rather use a binary encoding using ASN.1 BER), and in that case it's illegal to provide substring components if the value is empty.

However, I realize that it's much more likely for developers to be more familiar with the string representations of substring filters, and it's understandable for someone to think that filters are provided to the LDAP server using their string representation, and in that case, it may make sense to assume that providing an empty string is equivalent to not providing one at all.

I went ahead made a couple of changes to the API. I have updated the documentation to indicate that the components should be null or non-empty strings, but also to treat empty strings as equivalent to null in this case. That change should be included in the next release (currently slated for sometime in December), but the workaround, and more correct behavior, would be to use null instead of empty strings.