InMemoryDirectoryServer: Search: Missing result for certain order of attributes in OR filter with non-ASCII strings

githubert commented 2 years ago

Summary

It seems I stumbled upon a very specific problem: If a search with an OR filter for a number of attributes is done, and the search term contains non-ASCII characters, it will fail if it is in the wrong order and tests a phone number attribute.

Affected version

SDK version: 6.0.3

Tests were done with OpenJDK version 11.0.13 (2021-10-19).

Expected outcome

I'd expect it to work in all cases as the equality check should just come up false, as far as I understand things — or at least not be order-dependent for the OR filter.

How to reproduce

I narrowed it down to the JUnit 5 test seen below.

The search is done twice with the same attributes, but once in the order sn, (value of attributeName), givenName, and the second time in the order givenName, (value of attributeName), sn.

This all runs fine for the name Doe, but the other names will fail on the second variant if there is a equality check involving a phone number attribute (like mobile, pager, homePhone, etc).

import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
import com.unboundid.ldap.sdk.Attribute;
import com.unboundid.ldap.sdk.Entry;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldap.sdk.SearchResult;
import com.unboundid.ldap.sdk.SearchScope;
import com.unboundid.util.LDAPTestUtils;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

import static com.unboundid.util.LDAPTestUtils.generateUserEntry;
import static org.junit.jupiter.api.Assertions.assertEquals;

public class SearchProblemTest {
    @ParameterizedTest
    @CsvSource({
            "Doe, mobile",
            "Doe, telephoneNumber",
            "Doe, displayName",
            "Doe, initials",
            "Müller, mobile", // Fails
            "Müller, telephoneNumber", // Fails
            "Müller, displayName",
            "Müller, initials",
            "Иванов, telephoneNumber", // Fails
            "Иванов, displayName",
            "Novák, telephoneNumber", // Fails
            "Novák, displayName",
    })
    void searchDependsOnOrder(String name, String additionalAttribute) throws LDAPException {
        InMemoryDirectoryServer directoryServer;
        InMemoryDirectoryServerConfig config = new InMemoryDirectoryServerConfig("dc=example,dc=com");

        directoryServer = new InMemoryDirectoryServer(config);
        directoryServer.add(LDAPTestUtils.generateDomainEntry("example", "dc=com"));

        directoryServer.startListening();

        Entry person = generateUserEntry("1", "dc=example,dc=com", "FirstName", name, "Password",
                new Attribute(additionalAttribute, "1234"));

        directoryServer.add(person);

        // Always checks out fine:
        SearchResult r1 = directoryServer.search("dc=example,dc=com", SearchScope.SUB,
                String.format("(&(|(objectClass=person)(objectClass=inetOrgPerson))" +
                        "(|(sn=%s)(%s=%s)(givenName=%s)))", name, additionalAttribute, name, name));

        System.out.println("First variant:");
        r1.getSearchEntries().forEach(System.out::println);
        assertEquals(1, r1.getEntryCount());

        // Fails sometimes:
        SearchResult r2 = directoryServer.search("dc=example,dc=com", SearchScope.SUB,
                String.format("(&(|(objectClass=person)(objectClass=inetOrgPerson))" +
                        "(|(givenName=%s)(%s=%s)(sn=%s)))", name, additionalAttribute, name, name));

        System.out.println("Second variant:");
        r2.getSearchEntries().forEach(System.out::println);
        assertEquals(1, r2.getEntryCount());

        directoryServer.shutDown(true);
    }
}

dirmgr commented 2 years ago

Thanks for reporting this. I can confirm a bug in the Filter.matchesEntry method for the case in which one of the filter components was invalid.

The Filter.matchesEntry method will throw an exception when provided with a filter of "(telephoneNumber=Novák)", at least in the case when it has knowledge of a schema that knows the syntax for the telephoneNumber attribute, and that exception will have a result code of 21 (invalidAttributeSyntax) because valid values of that syntax are only allowed to have printable ASCII characters, and the "Latin small letter A with acute" character isn't allowed by that syntax. This is the correct behavior for that simple equality filter. However, when that equality filter is contained inside an AND or an OR, then that invalid filter should be treated as a non-match rather than rejecting the entire filter.

The reason that you were seeing different behavior when providing the filter components in a different order lies in the way that the filter evaluation is performed. For an OR filter, at least one of the components needs to match, and the LDAP SDK evaluates them in left-to-right. It will return true as soon as it finds a that matches, and it will return false if it gets through all of the components without finding a match. In your case, if you provide a matching component as the first one inside the OR, then it will identify the match and return true right away without considering any of the other components. But if the matching component is after the invalid component, there would have been an exception before getting to the matching component, and the in-memory directory server would also have treated that as no match.

I have committed a fix for the problem. It will be addressed in the next release, which will probably be in a couple of months. If you'd like a fix before then, you can build the LDAP SDK for yourself. Alternatively, I believe that it should be possible to work around the issue in your test by running the in-memory directory server without any schema (in which case it will assume a generic directory string syntax for all attributes with case-insensitive matching). You can do that by invoking the InMemoryDirectoryServerConfig.setSchema method with a value of null.

githubert commented 2 years ago

Thank you very much for the quick reply and detailed analysis, I'm very happy to hear that this will be fixed in the future.

I think I can work out a solution based on your suggestions for the tests and knowing how it is handled, also for the production code that this relates to (if it is affected at all). Looking forward to the next release :)

pingidentity / ldapsdk