#50252 Ticket 49372 - filter optimisation improvements for common queries
Opened a year ago by firstyear. Modified a year ago
firstyear/389-ds-base 50073-enable-filter-opt  into  master

file modified
+1

@@ -2132,6 +2132,7 @@ 

  test_slapd_SOURCES = test/main.c \

  	test/libslapd/test.c \

  	test/libslapd/counters/atomic.c \

+ 	test/libslapd/filter/optimise.c \

  	test/libslapd/pblock/analytics.c \

  	test/libslapd/pblock/v3_compat.c \

  	test/libslapd/operation/v3_compat.c \

@@ -1,12 +1,15 @@ 

  # --- BEGIN COPYRIGHT BLOCK ---

  # Copyright (C) 2019 RED Hat, Inc.

+ # Copyright (C) 2019 William Brown <william@blackhats.net.au>

  # All rights reserved.

  #

  # License: GPL (version 3 or any later version).

  # See LICENSE for details.

  # --- END COPYRIGHT BLOCK ----

  

- import pytest, os

+ import ldap

+ import pytest

+ import os

  

  from lib389._constants import DEFAULT_SUFFIX, PW_DM

  from lib389.topologies import topology_st as topo

@@ -28,20 +31,22 @@ 

          1. Entry should be added

          2. Operation should  succeed

      """

-     user = UserAccounts(topo.standalone, DEFAULT_SUFFIX)

+     users = UserAccounts(topo.standalone, DEFAULT_SUFFIX)

      for i in range(1, 5):

-         user1 = user.create_test_user(uid=i)

-         user1.set("mail", "AnujBorah{}@ok.com".format(i))

+         user = users.create_test_user(uid=i)

+         user.set("mail", "email{}@example.com".format(i))

  

-     # Testing filter is working for any king of attr

+     # Testing users filter is working for any kind of attr

  

-     user = Accounts(topo.standalone, DEFAULT_SUFFIX)

- 

-     assert len(user.filter('(mail=*)')) == 4

-     assert len(user.filter('(uid=*)')) == 4

+     assert len(users.filter('(mail=*)')) == 4

+     assert len(users.filter('(uid=*)')) == 4

  

      # Testing filter is working for other filters

-     assert len(user.filter("(objectclass=inetOrgPerson)")) == 4

+     assert len(users.filter("(objectclass=nsAccount)")) == 4

+ 

+     # Test that invalid filter fails (note the missing ()"

+     with pytest.raises(ldap.FILTER_ERROR):

+         users.filter("objectclass=nsAccount")

  

  

  if __name__ == "__main__":

@@ -1,3 +1,11 @@ 

+ # --- BEGIN COPYRIGHT BLOCK ---

+ # Copyright (C) 2019 William Brown <william@blackhats.net.au>

+ # All rights reserved.

+ #

+ # License: GPL (version 3 or any later version).

+ # See LICENSE for details.

+ # --- END COPYRIGHT BLOCK ----

+ 

  import logging

  import pytest

  import os

@@ -16,6 +24,8 @@ 

  log = logging.getLogger(__name__)

  ALL_FILTERS = []

  

+ OU_PEOPLE = f"ou=People,{DEFAULT_SUFFIX}"

+ 

  

  # Parameterized filters to test

  AND_FILTERS = [("(&(uid=uid1)(sn=last1)(givenname=first1))", 1),

@@ -30,7 +40,9 @@ 

  OR_FILTERS = [("(|(uid=uid1)(sn=last1)(givenname=first1))", 1),

                ("(|(uid=uid1)(|(sn=last1)(givenname=first1)))", 1),

                ("(|(uid=uid1)(|(|(sn=last1))(|(givenname=first1))))", 1),

-               ("(|(objectclass=*)(sn=last1)(|(givenname=first1)))", 14),

+                 # This returns 6, not 5, due to SCOPE_SUBTREE, and oc=* collects

+                 # the ou due to OR condition.

+               ("(|(objectclass=*)(sn=last1)(|(givenname=first1)))", 6),

                ("(|(&(objectclass=*)(sn=last1))(|(givenname=first1)))", 1),

                ("(|(&(objectclass=*)(sn=last))(|(givenname=first1)))", 1)]

  

@@ -87,9 +99,13 @@ 

  

  @pytest.fixture(scope="module")

  def setup(topo, request):

-     """Add teset users

+     """Add test users

      """

  

+     # First, purge the subtree of any demo accounts we may have created

+     # as part of the setup process.

+     topo.standalone.delete_branch_s(OU_PEOPLE, ldap.SCOPE_ONELEVEL)

+ 

      users = UserAccounts(topo.standalone, DEFAULT_SUFFIX)

      for i in range(1, 6):

          users.create(properties={

@@ -106,7 +122,8 @@ 

  @pytest.mark.parametrize("myfilter, expected_results", ALL_FILTERS)

  def test_filters(topo, setup, myfilter, expected_results):

      """Test various complex search filters and verify they are returning the

-     expected number of entries

+     expected number of entries. We limit this to OU_PEOPLE, because we can't

+     guarantee the version of the DB we have, and what entries also exist.

  

      :id: ee9ead27-5f63-4aed-844d-c39b99138c8d

      :setup: standalone

@@ -120,7 +137,7 @@ 

  

      log.info("Testing filter \"{}\"...".format(myfilter))

      try:

-         entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, myfilter)

+         entries = topo.standalone.search_s(OU_PEOPLE, ldap.SCOPE_SUBTREE, myfilter)

          if len(entries) != expected_results:

              log.fatal("Search filter \"{}\") returned {} entries, but we expected {}".format(

                  myfilter, len(entries), expected_results))

@@ -1,5 +1,6 @@ 

  # --- BEGIN COPYRIGHT BLOCK ---

  # Copyright (C) 2017 Red Hat, Inc.

+ # Copyright (C) 2019 William Brown <william@blackhats.net.au>

  # All rights reserved.

  #

  # License: GPL (version 3 or any later version).

@@ -11,7 +12,7 @@ 

  import ldap

  

  from lib389.topologies import topology_st

- from lib389._constants import DEFAULT_SUFFIX

+ from lib389._constants import DEFAULT_SUFFIX, ErrorLog

  

  from lib389.idm.user import UserAccounts

  

@@ -26,6 +27,7 @@ 

  works, where as most of these actually hit the filtertest threshold so they early return.

  """

  

+ OU_PEOPLE = f"ou=People,{DEFAULT_SUFFIX}"

  USER0_DN = 'uid=user0,ou=People,%s' % DEFAULT_SUFFIX

  USER1_DN = 'uid=user1,ou=People,%s' % DEFAULT_SUFFIX

  USER2_DN = 'uid=user2,ou=People,%s' % DEFAULT_SUFFIX

@@ -49,6 +51,10 @@ 

  

  @pytest.fixture(scope="module")

  def topology_st_f(topology_st):

+     # First, purge the subtree of any demo accounts we may have created

+     # as part of the setup process.

+     topology_st.standalone.delete_branch_s(OU_PEOPLE, ldap.SCOPE_ONELEVEL)

+ 

      # Add our users to the topology_st

      users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX)

      for i in range(0, 20):

@@ -58,19 +64,31 @@ 

              'sn': '%s' % i,

              'uidNumber': '%s' % i,

              'gidNumber': '%s' % i,

-             'homeDirectory': '/home/user%s' % i

+             'homeDirectory': '/home/user%s' % i,

+             'description': 'test',

          })

      # return it

      # print("ATTACH NOW")

      # import time

      # time.sleep(30)

+ 

+     # Setup logging.

+     # topology_st.standalone.config.loglevel([ErrorLog.DEFAULT, ErrorLog.SEARCH_FILTER])

+ 

      return topology_st.standalone

  

- def _check_filter(topology_st_f, filt, expect_len, expect_dns):

-     # print("checking %s" % filt)

-     results = topology_st_f.search_s("ou=People,%s" % DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL, filt, ['uid',])

+ def _check_filter(topology_st_f, filt, expect_len, expect_dns, scope=ldap.SCOPE_ONELEVEL):

+     print("++++++++++ checking %s" % filt)

+ 

+     results = topology_st_f.search_s("ou=People,%s" % DEFAULT_SUFFIX, scope, filt, ['uid',])

+     # Because LDAP allows case-sensitive in dns, but the ldap compare

+     # is case-insensitive, we have to normalise our checks.

+     result_dns = [result.dn.lower() for result in results]

+     expect_dns = [expect.lower() for expect in expect_dns]

+     print(sorted(result_dns))

+     print(sorted(expect_dns))

+ 

      assert len(results) == expect_len

-     result_dns = [result.dn for result in results]

      assert set(expect_dns) == set(result_dns)

  

  

@@ -444,4 +462,83 @@ 

      _check_filter(topology_st_f, '(|(&(uid=user0)(sn=1))(uid=user0))', 1, [USER0_DN])

      _check_filter(topology_st_f, '(|(&(uid=user1)(sn=1))(uid=user0))', 2, [USER0_DN, USER1_DN])

  

+ def test_filter_shortcut_correct(topology_st):

+     """

+     Check that with an AND condition on shortcut, we correctly apply the filter

+     test to the candidate set.

+     """

+     # First, purge the subtree of any demo accounts we may have created

+     # as part of the setup process.

+     topology_st.standalone.delete_branch_s(OU_PEOPLE, ldap.SCOPE_ONELEVEL)

+ 

+     # Add our user to the topology_st: It has to be a single user to keey objectClass=posixAccount

+     # below filter test to emulate the IPA bug.

+     users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX)

+     users.create(properties={

+         'uid': 'user1',

+         'cn': 'user1',

+         'sn': '1',

+         'uidNumber': '1',

+         'gidNumber': '1',

+         'homeDirectory': '/home/user1',

+         'description': 'test',

+     })

+     # return it

+     # print("ATTACH NOW")

+     # import time

+     # time.sleep(30)

+ 

+     # Setup logging.

+     # topology_st.standalone.config.loglevel([ErrorLog.DEFAULT, ErrorLog.SEARCH_FILTER])

+ 

+     #                                   V-- should become allIDS

+     #                                                                  V-- should be less than test threshold

+     _check_filter(topology_st.standalone, '(&(|(uid=user1)(description=test))(objectClass=posixAccount)(uid=*)(objectClass=group))', 0, [], ldap.SCOPE_SUBTREE)

+ 

+ 

+ def test_ipa_cert_filter_complex(topology_st_f):

+     """

+     IPA generates a number of interesting filters, that have historically broken filter

+     optimisation. This is a pair of those for testing.

+ 

+     working

+     a = "(&

+             (|

+                 (usercertificate;binary=0\82\03\AD0\82\02\95\A0\03\02\01\02\02\01\130\0D\06\09\2A\86H\86\F7\0D\01\01\0B\05)

+                 (ipaCertMapData=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1)

+             )

+             (objectClass=posixAccount)

+             (uid=*)

+             (&

+                 (uidNumber=*)

+                 (!

+                     (uidNumber=0)

+                 )

+             )

+         )"

+     failing

+     b = "(&

+             (|

+                 (usercertificate;binary=0\82\03\AD0\82\02\95\A0\03\02\01\02\02\01\130\0D\06\09\2A\86H\86\F7\0D\01\01\0B\05)

+                 (ipaCertMapData=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1)

+                 (altsecurityidentities=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1)

+             )

+             (objectClass=posixAccount)

+             (uid=*)

+             (&

+                 (uidNumber=*)

+                 (!

+                     (uidNumber=0)

+                 )

+             )

+         )"

+ 

+     """

+ 

+     results = topology_st_f.search_s(DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL, "(&(|(usercertificate;binary=0\\82\\03\\AD0\\82\\02\\95\\A0\\03\\02\\01\\02\\02\\01\\130\\0D\\06\\09\\2A\\86H\\86\\F7\\0D\\01\\01\\0B\\05)(ipaCertMapData=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1))(objectClass=posixAccount)(uid=*)(&(uidNumber=*)(!(uidNumber=0))))", ['uid',])

+     results = topology_st_f.search_s(DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL, "(&(|(usercertificate;binary=0\\82\\03\\AD0\\82\\02\\95\\A0\\03\\02\\01\\02\\02\\01\\130\\0D\\06\\09\\2A\\86H\\86\\F7\\0D\\01\\01\\0B\\05)(ipaCertMapData=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1)(altsecurityidentities=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1))(objectClass=posixAccount)(uid=*)(&(uidNumber=*)(!(uidNumber=0))))", ['uid',])

+ 

+     results = topology_st_f.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(&(|(usercertificate;binary=0\\82\\03\\AD0\\82\\02\\95\\A0\\03\\02\\01\\02\\02\\01\\130\\0D\\06\\09\\2A\\86H\\86\\F7\\0D\\01\\01\\0B\\05)(ipaCertMapData=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1))(objectClass=posixAccount)(uid=*)(&(uidNumber=*)(!(uidNumber=0))))", ['uid',])

+     results = topology_st_f.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(&(|(usercertificate;binary=0\\82\\03\\AD0\\82\\02\\95\\A0\\03\\02\\01\\02\\02\\01\\130\\0D\\06\\09\\2A\\86H\\86\\F7\\0D\\01\\01\\0B\\05)(ipaCertMapData=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1)(altsecurityidentities=X509:<I>O=TESTRELM.TEST,CN=Certificate Authority<S>O=TESTRELM.TEST,CN=ipauser1))(objectClass=posixAccount)(uid=*)(&(uidNumber=*)(!(uidNumber=0))))", ['uid',])

+ 

  

@@ -114,7 +114,7 @@ 

      :id: cf5a6078-bbe6-4d43-ac71-553c45923f91

      :setup: Standalone instance

      :steps:

-          1. Search cn=Directory Administrators,dc=example,dc=com using ldapsearch with

+          1. Search ou=people,dc=example,dc=com using ldapsearch with

              scope one using base as dc=example,dc=com

           2. Check that search should return only one entry

      :expectedresults:

@@ -122,11 +122,8 @@ 

           2. This should pass

      """

  

-     parent_dn="dn: dc=example,dc=com"

-     child_dn="dn: cn=Directory Administrators,dc=example,dc=com"

- 

      log.info('Search user using ldapsearch with scope one')

-     results = topology_st.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL,'cn=Directory Administrators',['cn'] )

+     results = topology_st.standalone.search_s('ou=People,%s' % DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL,'ou=People',['ou'])

      log.info(results)

  

      log.info('Search should only have one entry')

@@ -1,5 +1,6 @@ 

  # --- BEGIN COPYRIGHT BLOCK ---

  # Copyright (C) 2016 Red Hat, Inc.

+ # Copyright (C) 2019 William Brown <william@blackhats.net.au>

  # All rights reserved.

  #

  # License: GPL (version 3 or any later version).

@@ -11,30 +12,34 @@ 

  from lib389.utils import *

  from lib389.topologies import topology_st

  from lib389.idm.user import UserAccounts

+ from lib389.idm.account import Account

+ from lib389.idm.directorymanager import DirectoryManager

+ from lib389.idm.domain import Domain

  

  from lib389._constants import DN_DM, DEFAULT_SUFFIX, DN_CONFIG, PASSWORD

  

  pytestmark = pytest.mark.tier1

  

  DN_PEOPLE = 'ou=people,%s' % DEFAULT_SUFFIX

- DN_ROOT = ''

+ # The RootDSE entry has dn of ''

+ DN_DSE = ''

  TEST_USER_NAME = 'all_attrs_test'

  TEST_USER_DN = 'uid=%s,%s' % (TEST_USER_NAME, DN_PEOPLE)

  TEST_USER_PWD = 'all_attrs_test'

  

  # Suffix for search, Regular user boolean, List of expected attrs

- TEST_PARAMS = [(DN_ROOT, False, [

+ TEST_PARAMS = [(DN_DSE, False, [

                  'aci', 'createTimestamp', 'creatorsName',

                  'modifiersName', 'modifyTimestamp', 'namingContexts',

-                 'nsBackendSuffix', 'nsUniqueId', 'subschemaSubentry',

+                 'nsBackendSuffix', 'subschemaSubentry',

                  'supportedControl', 'supportedExtension',

                  'supportedFeatures', 'supportedLDAPVersion',

                  'supportedSASLMechanisms', 'vendorName', 'vendorVersion'

  ]),

-                (DN_ROOT, True, [

+                (DN_DSE, True, [

                  'createTimestamp', 'creatorsName',

                  'modifiersName', 'modifyTimestamp', 'namingContexts',

-                 'nsBackendSuffix', 'nsUniqueId', 'subschemaSubentry',

+                 'nsBackendSuffix', 'subschemaSubentry',

                  'supportedControl', 'supportedExtension',

                  'supportedFeatures', 'supportedLDAPVersion',

                  'supportedSASLMechanisms', 'vendorName', 'vendorVersion'

@@ -44,20 +49,16 @@ 

                  'entryid', 'modifiersName', 'modifyTimestamp',

                  'nsUniqueId', 'numSubordinates', 'parentid'

                 ]),

-                (DN_PEOPLE, True, [

-                 'createTimestamp', 'creatorsName', 'entrydn',

-                 'entryid', 'modifyTimestamp', 'nsUniqueId',

-                 'numSubordinates', 'parentid'

-                ]),

+                # In 1.4.0, you can read ou/oc, but due to the design of this test

+                # which is testing the "+" behaviour, we only get oc in the * test.

+                # IE we see nothing besid OC in this test (because ou is *, not +)

+                (DN_PEOPLE, True, []),

                 (TEST_USER_DN, False, [

                  'createTimestamp', 'creatorsName', 'entrydn',

                  'entryid', 'modifiersName', 'modifyTimestamp',

                  'nsUniqueId', 'parentid'

                 ]),

-                (TEST_USER_DN, True, [

-                 'createTimestamp', 'creatorsName', 'entrydn',

-                 'entryid', 'modifyTimestamp', 'nsUniqueId', 'parentid'

-                ]),

+                (TEST_USER_DN, True, ['nsUniqueId']),

                 (DN_CONFIG, False, [

                  'numSubordinates', 'passwordHistory'

                 ])

@@ -85,13 +86,13 @@ 

      """Don't allow modifiersName attribute for the test user

      under whole suffix

      """

- 

-     ACI_TARGET = '(targetattr= "modifiersName")'

-     ACI_RULE = ('(version 3.0; acl "Deny modifiersName for user"; deny (read)'

-                 ' userdn = "ldap:///%s";)' % TEST_USER_DN)

-     ACI_BODY = ensure_bytes(ACI_TARGET + ACI_RULE)

-     topology_st.standalone.modify_s(DEFAULT_SUFFIX, [(ldap.MOD_ADD, 'aci', ACI_BODY)])

- 

+     d = Domain(topology_st.standalone, DEFAULT_SUFFIX)

+     planned_aci = f'(targetattr= "modifiersName")(version 3.0; acl "Deny modifiersName for user"; deny (read) userdn = "ldap:///{TEST_USER_DN}";)'

+     print("++++++++++++++++++++ %s" % planned_aci)

+     d.add(

+         'aci',

+         planned_aci

+     )

  

  def test_supported_features(topology_st):

      """Verify that OID 1.3.6.1.4.1.4203.1.5.1 is published

@@ -113,8 +114,8 @@ 

      assert supported_value == [b'1.3.6.1.4.1.4203.1.5.1']

  

  

- @pytest.mark.parametrize('add_attr', ['', '*', 'objectClass'])

- @pytest.mark.parametrize('search_suffix,regular_user,oper_attr_list',

+ @pytest.mark.parametrize('add_attr', [None, '*', 'objectClass'])

+ @pytest.mark.parametrize('search_suffix, regular_user, oper_attr_list',

                           TEST_PARAMS)

  def test_search_basic(topology_st, create_user, user_aci, add_attr,

                        search_suffix, regular_user, oper_attr_list):

@@ -139,28 +140,43 @@ 

           3. It should pass

      """

  

+     conn = None

+ 

      if regular_user:

          log.info("bound as: %s", TEST_USER_DN)

-         topology_st.standalone.simple_bind_s(TEST_USER_DN, ensure_bytes(TEST_USER_PWD))

+         conn = Account(topology_st.standalone, dn=TEST_USER_DN).bind(TEST_USER_PWD)

      else:

-         log.info("bound as: %s", DN_DM)

-         topology_st.standalone.simple_bind_s(DN_DM, ensure_bytes(PASSWORD))

+         log.info("Binding as: %s", DN_DM)

+         conn = DirectoryManager(topology_st.standalone).bind()

+ 

+     assert conn is not None

  

-     search_filter = ['+']

+     requested_attributes = ['+']

      expected_attrs = oper_attr_list

-     if add_attr:

-         search_filter.append(add_attr)

+     if add_attr is not None:

+         requested_attributes.append(add_attr)

          expected_attrs += ['objectClass']

  

-     entries = topology_st.standalone.search_s(search_suffix, ldap.SCOPE_BASE,

+     print('==================')

+     print(search_suffix)

+     print(regular_user)

+     print(requested_attributes)

+     print(set(expected_attrs))

+ 

+     entries = conn.search_s(search_suffix, ldap.SCOPE_BASE,

                                                '(objectclass=*)',

-                                               search_filter)

+                                               requested_attributes)

      found_attrs = entries[0].data.keys()

  

-     if add_attr == '*':

-         assert set(expected_attrs) - set(found_attrs) == set()

-     else:

-         assert set(expected_attrs) == set(found_attrs)

+     print(set(found_attrs))

+     print('==================')

+ 

+ 

+     assert set(expected_attrs).issubset(set(found_attrs))

+     # if add_attr == '*':

+     #     assert set(expected_attrs) - set(found_attrs) == set()

+     # else:

+     #     assert set(expected_attrs) == set(found_attrs)

  

  

  if __name__ == '__main__':

@@ -0,0 +1,272 @@ 

+ # --- BEGIN COPYRIGHT BLOCK ---

+ # Copyright (C) 2015 Red Hat, Inc.

+ # Copyright (C) 2019 William Brown <william@blackhats.net.au>

+ # All rights reserved.

+ #

+ # License: GPL (version 3 or any later version).

+ # See LICENSE for details. 

+ # --- END COPYRIGHT BLOCK ---

+ #

+ import os

+ import sys

+ import time

+ import ldap

+ import logging

+ import pytest

+ from lib389 import DirSrv, Entry, tools

+ from lib389.tools import DirSrvTools

+ from lib389._constants import *

+ from lib389.properties import *

+ from lib389.idm.domain import Domain

+ 

+ from lib389.topologies import topology_st as topology

+ 

+ log = logging.getLogger(__name__)

+ 

+ installation_prefix = None

+ 

+ USER_NUM = 10

+ TEST_USER = 'user'

+ 

+ OTHER_NAME = 'other_entry'

+ MAX_OTHERS = 10

+ 

+ BIND_NAME  = 'bind_entry'

+ BIND_DN    = 'cn=%s, %s' % (BIND_NAME, SUFFIX)

+ BIND_PW    = 'password'

+ 

+ ENTRY_NAME = 'test_entry'

+ ENTRY_DN   = 'cn=%s, %s' % (ENTRY_NAME, SUFFIX)

+ ENTRY_OC   = "top person"

+ 

+ # subtrees used in test 

+ SUBTREE_BASE    = "ou=testbase,%s" %SUFFIX

+ SUBTREE_GREEN_1 = "ou=green_one,%s" % SUBTREE_BASE

+ SUBTREE_GREEN_2 = "ou=green_two,%s" % SUBTREE_BASE

+ SUBTREE_RED     = "ou=red,%s" % SUBTREE_BASE

+ SUBTREES = (SUBTREE_GREEN_1, SUBTREE_GREEN_2, SUBTREE_RED)

+ 

+ @pytest.fixture(scope="module")

+ def ticket48275_init(topology):

+     """

+         It creates identical entries in 3 subtrees

+         It creates aci which allow access to a set of attrs

+             in two of these subtrees for bound users

+         It creates a user to be used for test

+ 

+     """

+ 

+ 

+     topology.standalone.log.info("Add subtree base: %s" % SUBTREE_BASE)

+     topology.standalone.add_s(Entry((SUBTREE_BASE, {

+                                             'objectclass': "top organizationalunit".split(),

+                                             'ou': "testbase"})))

+     topology.standalone.log.info("Add subtree: %s" % SUBTREE_GREEN_1)

+     topology.standalone.add_s(Entry((SUBTREE_GREEN_1, {

+                                             'objectclass': "top organizationalunit".split(),

+                                             'ou': "green_one"})))

+     topology.standalone.log.info("Add subtree: %s" % SUBTREE_GREEN_2)

+     topology.standalone.add_s(Entry((SUBTREE_GREEN_2, {

+                                             'objectclass': "top organizationalunit".split(),

+                                             'ou': "green_two"})))

+     topology.standalone.log.info("Add subtree: %s" % SUBTREE_RED)

+     topology.standalone.add_s(Entry((SUBTREE_RED, {

+                                             'objectclass': "top organizationalunit".split(),

+                                             'ou': "red"})))

+ 

+     # entry used to bind with

+     topology.standalone.log.info("Add %s" % BIND_DN)

+     topology.standalone.add_s(Entry((BIND_DN, {

+                                             'objectclass': "top person".split(),

+                                             'sn':           BIND_NAME,

+                                             'cn':           BIND_NAME,

+                                             'userpassword': BIND_PW})))

+ 

+     # enable acl error logging

+     # mod = [(ldap.MOD_REPLACE, 'nsslapd-errorlog-level', '128')]

+     # topology.standalone.modify_s(DN_CONFIG, mod)

+ 

+     # get rid of default ACIs

+     mod = [(ldap.MOD_DELETE, 'aci', None)]

+     topology.standalone.modify_s(SUFFIX, mod)

+ 

+     # Ok Now add the proper ACIs

+     ACI_TARGET       = "(target = \"ldap:///%s\")" % SUBTREE_GREEN_1

+     ACI_TARGETATTR   = "(targetattr = \"objectclass || cn || sn || uid || givenname\")"

+     ACI_ALLOW        = "(version 3.0; acl \"Allow search-read to green attrs\"; allow (read, search, compare)"

+     ACI_SUBJECT      = " userdn = \"ldap:///all\";)"

+     ACI_BODY         = ACI_TARGET + ACI_TARGETATTR + ACI_ALLOW + ACI_SUBJECT

+     # mod = [(ldap.MOD_ADD, 'aci', ACI_BODY)]

+     # topology.standalone.modify_s(SUFFIX, mod)

+     domain = Domain(topology.standalone, DEFAULT_SUFFIX)

+     domain.add('aci', ACI_BODY)

+ 

+     ACI_TARGET       = "(target = \"ldap:///%s\")" % SUBTREE_GREEN_2

+     ACI_TARGETATTR   = "(targetattr != \"employeenumber || mobile || telephonenumber || mail\")"

+     ACI_BODY         = ACI_TARGET + ACI_TARGETATTR + ACI_ALLOW + ACI_SUBJECT

+     # mod = [(ldap.MOD_ADD, 'aci', ACI_BODY)]

+     # topology.standalone.modify_s(SUFFIX, mod)

+     domain.add('aci', ACI_BODY)

+ 

+     log.info("Adding %d test entries..." % USER_NUM)

+     for id in range(USER_NUM):

+         name = "%s.%d" % (TEST_USER, id)

+         mail = "%s@example.com" % name

+         for subtree in SUBTREES:

+             topology.standalone.add_s(Entry(("cn=%s,%s" % (name, subtree), {

+                                          'objectclass': "top person organizationalPerson inetOrgPerson".split(),

+                                          'sn': name,

+                                          'cn': name,

+                                          'uid': name,

+                                          'givenname': 'test',

+                                          'mail': mail,

+                                          'description': 'description',

+                                          'employeenumber': "%d" % id,

+                                          'telephonenumber': "%d%d%d" % (id,id,id),

+                                          'mobile': "%d%d%d" % (id,id,id),

+                                          'l': 'MV',

+                                          'title': 'Engineer'})))

+ 

+ 

+ def gen_filtertests_basic(dm):

+     tests = []

+     # 1. Simple filter - green attr - not/matching

+     tests.append(("(uid=xxxx)",0))

+     tests.append(("(uid=user.1)",(3 if dm else 2)))

+ 

+     # 2. Simple filter -red attr - not/matching

+     tests.append(("(mail=user.x@example.com)",0))

+     tests.append(("(mail=user.1@example.com)",(3 if dm else 0)))

+ 

+     # 3. NOT filter -green attr - not/matching

+     tests.append(("(!(objectclass=inetorgperson))",(4 if dm else 2)))

+     tests.append(("(!(objectclass=dummy))",(34 if dm else 22)))

+ 

+     # 4. NOT filter - red attr - not/matching

+     tests.append(("(!(mail=user.1@example.com))",(31 if dm else 0)))

+     tests.append(("(!(mail=user.x@example.com))",(34 if dm else 0)))

+ 

+     # 5. AND filter - green/green attr - not/matching

+     tests.append(("(&(objectclass=inetorgperson)(uid=user.1))",(3 if dm else 2)))

+     tests.append(("(&(objectclass=inetorgperson)(uid=user.x))",0))

+ 

+     # 6. AND filter - green/red attr - not/matching

+     tests.append(("(&(objectclass=inetorgperson)(mail=user.1@example.com))",(3 if dm else 0)))

+     tests.append(("(&(objectclass=inetorgperson)(mail=user.x@example.com))",0))

+ 

+     # 7. OR filter - green/green - not/matching/order

+     tests.append(("(|(uid=user.1)(sn=user.3))",(6 if dm else 4)))

+     tests.append(("(|(uid=user.x)(sn=user.3))",(3 if dm else 2)))

+     tests.append(("(|(uid=user.1)(sn=user.x))",(3 if dm else 2)))

+     tests.append(("(|(uid=user.1)(mail=user.1@example.com))", (3 if dm else 2)))

+ 

+     return tests

+ 

+ def gen_filtertests_ext(dm):

+     '''

+     the extended tests add OR components to the search filter using attributes

+     without access, either matching or not.

+     The returned entries have to be the same as without these OR parts

+     '''

+ 

+     tests = []

+ 

+     # 1. Simple filter - green attr - not/matching

+     tests.append(("(|(uid=xxxx)(mail=user.xxxx1@example.com))",0))

+     tests.append(("(|(uid=xxxx)(mail=user.1@example.com))",(3 if dm else 0)))

+     tests.append(("(|(uid=user.1)(mail=user.xxxx1@example.com))",(3 if dm else 2)))

+     tests.append(("(|(uid=user.1)(mail=user.3@example.com))",(6 if dm else 2)))

+ 

+     # 2. Simple filter - red attr - not/matching

+     tests.append(("(|(mail=user.x@example.com)(employeenumber=3))",(3 if dm else 0)))

+     tests.append(("(|(mail=user.x@example.com)(employeenumber=100))",0))

+     tests.append(("(|(mail=user.1@example.com)(employeenumber=3))",(6 if dm else 0)))

+     tests.append(("(|(mail=user.1@example.com)(employeenumber=100))",(3 if dm else 0)))

+ 

+     # 3. NOT filter - green attr - not/matching

+     tests.append(("(!(|(objectclass=inetorgperson)(employeenumber=3)))",(4 if dm else 2)))

+     tests.append(("(!(|(objectclass=inetorgperson)(employeenumber=100)))",(4 if dm else 2)))

+     tests.append(("(|(!(objectclass=inetorgperson))(employeenumber=3))",(7 if dm else 2)))

+     tests.append(("(|(!(objectclass=inetorgperson))(employeenumber=100))",(4 if dm else 2)))

+ 

+     # 4. NOT filter - red attr - not/matching

+     tests.append(("(!(|(mail=user.1@example.com)(employeenumber=3)))",(28 if dm else 0)))

+     tests.append(("(!(|(mail=user.1@example.com)(employeenumber=100)))",(31 if dm else 0)))

+     tests.append(("(!(|(mail=user.x@example.com)(employeenumber=3)))",(31 if dm else 0)))

+     tests.append(("(!(|(mail=user.x@example.com)(employeenumber=100)))",(34 if dm else 0)))

+ 

+     # 5. AND filter - green/green attr - not/matching

+     tests.append(("(|(&(objectclass=inetorgperson)(uid=user.1))(employeenumber=3))",(6 if dm else 2)))

+     tests.append(("(|(&(objectclass=inetorgperson)(uid=user.1))(employeenumber=100))",(3 if dm else 2)))

+     tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=3)(uid=user.1)))",(6 if dm else 2)))

+     tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=100)(uid=user.1)))",(3 if dm else 2)))

+     tests.append(("(&(|(employeenumber=3)(objectclass=inetorgperson))(uid=user.1))",(3 if dm else 2)))

+     tests.append(("(&(|(employeenumber=100)(objectclass=inetorgperson))(uid=user.1))",(3 if dm else 2)))

+     tests.append(("(|(&(objectclass=inetorgperson)(uid=user.x))(employeenumber=3))",(3 if dm else 0)))

+     tests.append(("(|(&(objectclass=inetorgperson)(uid=user.x))(employeenumber=100))", 0))

+     tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=3)(uid=user.x)))",(3 if dm else 0)))

+     tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=100)(uid=user.x)))",0))

+     tests.append(("(&(|(employeenumber=3)(objectclass=inetorgperson))(uid=user.x))",0))

+     tests.append(("(&(|(employeenumber=100)(objectclass=inetorgperson))(uid=user.x))",0))

+ 

+     # 6. AND filter - green/red attr - not/matching

+     tests.append(("(&(|(employeenumber=3)(objectclass=inetorgperson))(mail=user.1@example.com))",(3 if dm else 0)))

+     tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=3)(mail=user.1@example.com)))",(6 if dm else 0)))

+     tests.append(("(|(employeenumber=3)(&(objectclass=inetorgperson)(mail=user.1@example.com)))",(6 if dm else 0)))

+     tests.append(("(&(|(employeenumber=100)(objectclass=inetorgperson))(mail=user.1@example.com))",(3 if dm else 0)))

+     tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=100)(mail=user.1@example.com)))",(3 if dm else 0)))

+     tests.append(("(|(employeenumber=100)(&(objectclass=inetorgperson)(mail=user.1@example.com)))",(3 if dm else 0)))

+     tests.append(("(&(objectclass=inetorgperson)(mail=user.x@example.com))",0))

+ 

+     # 7. OR filter - green/green - not/matching/order

+     tests.append(("(|(uid=user.1)(employeenumber=3)(sn=user.3))",(6 if dm else 4)))

+     tests.append(("(|(uid=user.1)(sn=user.3)(employeenumber=3))",(6 if dm else 4)))

+     tests.append(("(|(uid=user.x)(employeenumber=3)(sn=user.3))",(3 if dm else 2)))

+     tests.append(("(|(uid=user.x)(sn=user.3)(employeenumber=3))",(3 if dm else 2)))

+     tests.append(("(|(uid=user.1)(employeenumber=3)(sn=user.x))",(6 if dm else 2)))

+     tests.append(("(|(uid=user.1)(sn=user.x)(employeenumber=3))",(6 if dm else 2)))

+     tests.append(("(|(uid=user.1)(unknown=3)(sn=user.3))",(6 if dm else 4)))

+     tests.append(("(|(uid=user.1)(sn=user.3)(unknown=3))",(6 if dm else 4)))

+     tests.append(("(|(uid=user.x)(unknown=3)(sn=user.3))",(3 if dm else 2)))

+     tests.append(("(|(uid=user.x)(sn=user.3)(unknown=3))",(3 if dm else 2)))

+     tests.append(("(|(uid=user.1)(unknown=3)(sn=user.x))",(3 if dm else 2)))

+     tests.append(("(|(uid=user.1)(sn=user.x)(unknown=3))",(3 if dm else 2)))

+ 

+     return tests

+ 

+ def run_ticket48275_search(topology, dm, basic):

+ 

+     if basic:

+         tests = gen_filtertests_basic(dm)

+     else:

+         tests = gen_filtertests_ext(dm)

+ 

+     topology.standalone.log.info("\n\n######################### SEARCH ######################\n")

+ 

+     for (fstr, exp) in tests:

+         ents = topology.standalone.search_s(SUBTREE_BASE, ldap.SCOPE_SUBTREE, fstr)

+         rec = len(ents)

+         topology.standalone.log.info("Test: filter \"%s\", expected: %d, received: %d\n" % (fstr, exp, rec))

+         assert(exp == rec)

+ 

+ def test_ticket48275_search_basic_dm(topology, ticket48275_init):

+ 

+     topology.standalone.simple_bind_s(DN_DM, PASSWORD)

+     run_ticket48275_search(topology, dm=True, basic=True)

+ 

+ def test_ticket48275_search_basic_user(topology, ticket48275_init):

+ 

+     topology.standalone.simple_bind_s(BIND_DN, BIND_PW)

+     run_ticket48275_search(topology, dm=False, basic=True)

+ 

+ def test_ticket48275_search_ext_dm(topology, ticket48275_init):

+ 

+     topology.standalone.simple_bind_s(DN_DM, PASSWORD)

+     run_ticket48275_search(topology, dm=True, basic=False)

+ 

+ def test_ticket48275_search_ext_user(topology, ticket48275_init):

+ 

+     topology.standalone.simple_bind_s(BIND_DN, BIND_PW)

+     run_ticket48275_search(topology, dm=False, basic=False)

+ 

+ 

@@ -0,0 +1,63 @@ 

+ # --- BEGIN COPYRIGHT BLOCK ---

+ # Copyright (C) 2015 Red Hat, Inc.

+ # Copyright (C) 2019 William Brown <william@blackhats.net.au>

+ # All rights reserved.

+ #

+ # License: GPL (version 3 or any later version).

+ # See LICENSE for details. 

+ # --- END COPYRIGHT BLOCK ---

+ #

+ 

+ import logging

+ import pytest

+ import os

+ import ldap

+ import time

+ from lib389 import Entry

+ from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES

+ from lib389._constants import *

+ from lib389.topologies import topology_st as topo

+ 

+ TARGETATTR = "(targetattr = \"cn || createtimestamp || description || displayname || entryusn || gecos || gidnumber || givenname || homedirectory || initials || loginshell || manager || modifytimestamp || objectclass || sn || title || uid \")"

+ TARGETFILTER = "(targetfilter = \"(objectclass=posixaccount)\")"

+ RIGHTS = "(version 3.0;acl \"Read User Standard Attributes\";allow (compare,read,search) userdn = \"ldap:///anyone\";)"

+ ACI = TARGETATTR + TARGETFILTER + RIGHTS

+ 

+ PASSWD = 'password'

+ def test_ticket49617(topo):

+     """Specify a test case purpose or name here

+ 

+     :id: 0

+     :setup: Standalone Instance

+     :steps:

+         1. Fill in test case steps here

+         2. And indent them like this (RST format requirement)

+     :expectedresults:

+         1. Fill in the result that is expected

+         2. For each test step

+     """

+ 

+     S = topo.standalone

+ 

+     PEOPLE = "ou=People,%s" % SUFFIX

+     # Purge all system default aci

+     S.modify_s(SUFFIX, [(ldap.MOD_DELETE, 'aci', None)])

+     # Add the test ACI

+     S.modify_s(PEOPLE, [(ldap.MOD_REPLACE, 'aci', ACI.encode())])

+ 

+     uid = b'my_user'

+     users = UserAccounts(S, PEOPLE, rdn=None)

+     user_props = TEST_USER_PROPERTIES.copy()

+     user_props.update({'uid': uid, 'cn': uid, 'sn': '_%s' % uid, 'userpassword': PASSWD.encode(), 'description': b'value creation'})

+     test_user = users.create(properties=user_props)

+ 

+     Filter = "(uid=*)"

+ 

+     S.bind(test_user.dn, PASSWD)

+     ents = S.search_s(PEOPLE, ldap.SCOPE_SUBTREE, Filter)

+     # This is 2 because there is a demo user too now!

+     assert len(ents) == 2

+ 

+     ents = S.search_s(PEOPLE, ldap.SCOPE_ONELEVEL, Filter)

+     assert len(ents) == 2

+ 

@@ -16,9 +16,22 @@ 

  import itertools

  import re

  

+ from enum import IntEnum

+ 

  import gdb

  from gdb.FrameDecorator import FrameDecorator

  

+ class LDAPFilter(IntEnum):

+     PRESENT = 0x87

+     APPROX = 0xa8

+     LE = 0xa6

+     GE = 0xa5

+     SUBSTRINGS = 0xa4

+     EQUALITY = 0xa3

+     NOT = 0xa2

+     OR = 0xa1

+     AND = 0xa0

+ 

  class DSAccessLog (gdb.Command):

      """Display the Directory Server access log."""

      def __init__ (self):

@@ -163,9 +176,65 @@ 

          entry_attrs = entry_root['e_attrs']

          self._display_attrs(entry_attrs)

  

+ class DSFilterPrint (gdb.Command):

+     """Display a filter's contents"""

+     def __init__ (self):

+         super (DSFilterPrint, self).__init__ ("ds-filter-print", gdb.COMMAND_DATA)

+ 

+     def display_filter(self, filter_element, depth=0):

+         pad = " " * depth

+         # Extract the choice, that determines what we access next.

+         f_choice = filter_element['f_choice']

+         f_un = filter_element['f_un']

+         f_flags = filter_element['f_flags']

+         if f_choice == LDAPFilter.PRESENT:

+             print("%s(%s=*) flags:%s" % (pad, f_un['f_un_type'], f_flags))

+         elif f_choice == LDAPFilter.APPROX:

+             print("%sAPPROX ???" % pad)

+         elif f_choice == LDAPFilter.LE:

+             print("%sLE ???" % pad)

+         elif f_choice == LDAPFilter.GE:

+             print("%sGE ???" % pad)

+         elif f_choice == LDAPFilter.SUBSTRINGS:

+             f_un_sub = f_un['f_un_sub']

+             value = f_un_sub['sf_initial']

+             print("%s(%s=%s*) flags:%s" % (pad, f_un_sub['sf_type'], value, f_flags))

+         elif f_choice == LDAPFilter.EQUALITY:

+             f_un_ava = f_un['f_un_ava']

+             value = f_un_ava['ava_value']['bv_val']

+             print("%s(%s=%s) flags:%s" % (pad, f_un_ava['ava_type'], value, f_flags))

+         elif f_choice == LDAPFilter.NOT:

+             print("%sNOT ???" % pad)

+         elif f_choice == LDAPFilter.OR:

+             print("%s(| flags:%s" % (pad, f_flags))

+             filter_child = f_un['f_un_complex'].dereference()

+             self.display_filter(filter_child, depth + 4)

+             print("%s)" % pad)

+         elif f_choice == LDAPFilter.AND:

+             # Our child filter is in f_un_complex.

+             print("%s(& flags:%s" % (pad, f_flags))

+             filter_child = f_un['f_un_complex'].dereference()

+             self.display_filter(filter_child, depth + 4)

+             print("%s)" % pad)

+         else:

+             print("Corrupted filter, no such value %s" % f_choice)

+ 

+         f_next = filter_element['f_next']

+         if f_next != 0:

+             self.display_filter(f_next.dereference(), depth)

+ 

+     def invoke (self, arg, from_tty):

+         # Select our program state

+         gdb.newest_frame()

+         cur_frame = gdb.selected_frame()

+         # We are given the name of a filter, so we need to look up that symbol.

+         filter_val = cur_frame.read_var(arg)

+         filter_root = filter_val.dereference()

+         self.display_filter(filter_root)

+ 

  DSAccessLog()

  DSBacktrace()

  DSIdleFilter()

  DSEntryPrint()

- 

+ DSFilterPrint()

  

@@ -312,6 +312,7 @@ 

  attributeTypes: ( 2.16.840.1.113730.3.1.2344 NAME 'nsslapd-tls-check-crl' DESC 'Check CRL when opening outbound TLS connections. Valid options are none, peer, all.' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )

  attributeTypes: ( 2.16.840.1.113730.3.1.2353 NAME 'nsslapd-encryptionalgorithm' DESC 'The encryption algorithm used to encrypt the changelog' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )

  attributeTypes: ( 2.16.840.1.113730.3.1.2084 NAME 'nsSymmetricKey' DESC 'A symmetric key - currently used by attribute encryption' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X-ORIGIN 'attribute encryption' )

+ attributeTypes: ( 2.16.840.1.113730.3.1.2364 NAME 'nsslapd-filter-optimise-enable' DESC 'Enable or disable the query optimiser within the server' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )

  #

  # objectclasses

  #

@@ -82,6 +82,12 @@ 

          }

      }

  

+     if (slapi_is_loglevel_set(SLAPI_LOG_FILTER)) {

+         char fbuf[4096] = {0};

+         slapi_filter_to_string(f, fbuf, 4095);

+         slapi_log_err(SLAPI_LOG_FILTER, "filter_candidates_ext", "processing filt -> %s\n", fbuf);

+     }

+ 

      result = NULL;

      switch ((ftype = slapi_filter_get_choice(f))) {

      case LDAP_FILTER_EQUALITY:

@@ -144,8 +150,7 @@ 

          break;

      }

  

-     slapi_log_err(SLAPI_LOG_TRACE, "filter_candidates_ext", "<= %lu\n",

-                   (u_long)IDL_NIDS(result));

+     slapi_log_err(SLAPI_LOG_FILTER, "filter_candidates_ext", "<= IDL length(%lu)\n", (u_long)IDL_NIDS(result));

      return (result);

  }

  

@@ -631,7 +636,7 @@ 

      int is_and = 0;

      IDListSet *idl_set = NULL;

  

-     slapi_log_err(SLAPI_LOG_TRACE, "list_candidates", "=> 0x%x\n", ftype);

+     slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "=> 0x%x\n", ftype);

  

      /*

       * Optimize bounded range queries such as (&(cn>=A)(cn<=B)).

@@ -814,21 +819,31 @@ 

              idl_set_insert_complement_idl(idl_set, tmp);

          }

  

-         if (ftype == LDAP_FILTER_OR && idl_set_union_shortcut(idl_set) != 0) {

-             /*

-              * If we encounter an allids idl, this means that union will return

-              * and allids - we should not process anymore, and fallback to full

-              * table scan at this point.

-              */

-             goto apply_set_op;

+         if (ftype == LDAP_FILTER_OR) {

+             if (idl_set_union_shortcut(idl_set) != 0) {

+                 /*

+                  * If we encounter an allids idl, this means that union will return

+                  * and allids - we should not process anymore, and fallback to full

+                  * table scan at this point.

+                  */

+                 slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "union shortcut\n");

+                 goto apply_set_op;

+             } else {

+                 slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "NOT applying union shortcut\n");

+             }

          }

  

-         if (ftype == LDAP_FILTER_AND && idl_set_intersection_shortcut(idl_set) != 0) {

-             /*

-              * If we encounter a zero length idl, we bail now because this can never

-              * result in a meaningful result besides zero.

-              */

-             goto apply_set_op;

+         if (ftype == LDAP_FILTER_AND) {

+             if (idl_set_intersection_shortcut(idl_set) != 0) {

+                 /*

+                  * If we encounter a zero length idl, we bail now because this can never

+                  * result in a meaningful result besides zero.

+                  */

+                 slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "intersection shortcut (filter test threshold)\n");

+                 goto apply_set_op;

+             } else {

+                 slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "NOT applying intersection shortcut (filter test threshold)\n");

+             }

          }

      }

  

@@ -856,8 +871,7 @@ 

          idl = idl_set_intersect(idl_set, be);

      }

  

-     slapi_log_err(SLAPI_LOG_TRACE, "list_candidates", "<= %lu\n",

-                   (u_long)IDL_NIDS(idl));

+     slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "<= 0x%x IDL length(%lu)\n", ftype, (u_long)IDL_NIDS(idl));

  out:

      idl_set_destroy(idl_set);

      if (is_and) {

@@ -252,12 +252,16 @@ 

       * Check allids first, because allids = 1, may not

       * have set count > 0.

       */

-     if (idl_set->allids != 0) {

+     if (idl_set_union_shortcut(idl_set) != 0) {

          idl_set_free_idls(idl_set);

+         /* An OR condition was allids or unindexed, do not bypass */

+         slapi_be_set_flag(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST);

          return idl_allids(be);

      } else if (idl_set->count == 0) {

+         /* Empty union */

          return idl_alloc(0);

      } else if (idl_set->count == 1) {

+         /* Only one set, must be only this IDL */

          return idl_set->head;

      } else if (idl_set->count == 2) {

          IDList *result_list = idl_union(be, idl_set->head, idl_set->head->next);

@@ -350,7 +354,11 @@ 

      IDList *result_list = NULL;

  

      if (idl_set->allids) {

-         /* if any component was allids we have to apply the filtertest */

+         /*

+          * if any component was allids we have to apply the filtertest

+          * However, we can still continue as we DO reduce the cand set

+          * in this process.

+          */

          slapi_be_set_flag(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST);

      }

  

@@ -369,7 +377,11 @@ 

           * If allids, when we intersect head, we get head, so just skip that.

           */

          result_list = idl_set->head;

-     } else if (idl_set->minimum->b_nids <= FILTER_TEST_THRESHOLD) {

+     } else if (idl_set_intersection_shortcut(idl_set)) {

+         /*

+          * We return the small IDL here, because we then only need to inspect the

+          * smallest possible matching candidate set.

+          */

          result_list = idl_set->minimum;

          slapi_be_set_flag(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST);

  

@@ -2099,8 +2099,13 @@ 

           * being moved */

          strcpy(filterstr, "objectclass=*");

          filter = slapi_str2filter(filterstr);

+         /*

+          * We used to set managedSAIT here, but because the subtree create

+          * referral step is now in build_candidate_list, we can trust the filter

+          * we provide here is exactly as we provide it IE no referrals involved.

+          */

          candidates = subtree_candidates(pb, be, slapi_sdn_get_ndn(dn_parentdn),

-                                         parententry, filter, 1 /* ManageDSAIT */,

+                                         parententry, filter,

                                          NULL /* allids_before_scopingp */, &err);

          slapi_filter_free(filter, 1);

      }

@@ -31,10 +31,10 @@ 

  /* prototypes */

  static int build_candidate_list(Slapi_PBlock *pb, backend *be, struct backentry *e, const char *base, int scope, int *lookup_returned_allidsp, IDList **candidates);

  static IDList *base_candidates(Slapi_PBlock *pb, struct backentry *e);

- static IDList *onelevel_candidates(Slapi_PBlock *pb, backend *be, const char *base, struct backentry *e, Slapi_Filter *filter, int managedsait, int *lookup_returned_allidsp, int *err);

+ static IDList *onelevel_candidates(Slapi_PBlock *pb, backend *be, const char *base, Slapi_Filter *filter, int *lookup_returned_allidsp, int *err);

  static back_search_result_set *new_search_result_set(IDList *idl, int vlv, int lookthroughlimit);

  static void delete_search_result_set(Slapi_PBlock *pb, back_search_result_set **sr);

- static int can_skip_filter_test(Slapi_PBlock *pb, struct slapi_filter *f, int scope, IDList *idl);

+ static int can_skip_filter_test(Slapi_PBlock *pb, backend *be, struct slapi_filter *f, int scope, IDList *idl);

  

  /* This is for performance testing, allows us to disable ACL checking altogether */

  #if defined(DISABLE_ACL_CHECK)

@@ -881,7 +881,7 @@ 

              tmp_desc = "Filter is not set";

              goto bail;

          }

-         if (can_skip_filter_test(pb, filter, scope, candidates)) {

+         if (can_skip_filter_test(pb, be, filter, scope, candidates)) {

              sr->sr_flags |= SR_FLAG_CAN_SKIP_FILTER_TEST;

          }

      }

@@ -940,16 +940,18 @@ 

      struct ldbminfo *li = (struct ldbminfo *)be->be_database->plg_private;

      int managedsait = 0;

      Slapi_Filter *filter = NULL;

+     Slapi_Filter *filter_original = NULL;

      int err = 0;

      int r = 0;

+     int32_t filter_optimise = 0;

  

-     slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter);

-     if (NULL == filter) {

+     slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter_original);

+ 

+     if (NULL == filter_original) {

          slapi_send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL, "No filter", 0, NULL);

          r = SLAPI_FAIL_GENERAL;

          goto bail;

      }

- 

      slapi_pblock_get(pb, SLAPI_MANAGEDSAIT, &managedsait);

  

      switch (scope) {

@@ -958,13 +960,32 @@ 

          break;

  

      case LDAP_SCOPE_ONELEVEL:

-         *candidates = onelevel_candidates(pb, be, base, e, filter, managedsait,

-                                           lookup_returned_allidsp, &err);

+         /* Duplicate so we don't tamper with the original. */

+         filter = slapi_filter_dup(filter_original);

+         filter_optimise = slapi_pblock_get_filter_optimise_enable(pb);

+ 

+         /* modify the filter to be: (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) */

+         filter = create_onelevel_filter(filter, e, managedsait);

+         /* Now optimise the filter for use */

+         slapi_filter_optimise(filter, filter_optimise);

+ 

+         *candidates = onelevel_candidates(pb, be, base, filter, lookup_returned_allidsp, &err);

+         /* Give the optimised filter back to search filter for free */

+         slapi_pblock_set_filter_executed(pb, filter);

          break;

  

      case LDAP_SCOPE_SUBTREE:

-         *candidates = subtree_candidates(pb, be, base, e, filter, managedsait,

-                                          lookup_returned_allidsp, &err);

+         /* Duplicate so we don't tamper with the original. */

+         filter = slapi_filter_dup(filter_original);

+         filter_optimise = slapi_pblock_get_filter_optimise_enable(pb);

+         /* make (|(originalfilter)(objectclass=referral)) */

+         filter = create_subtree_filter(filter, managedsait);

+         /* Now optimise the filter for use */

+         slapi_filter_optimise(filter, filter_optimise);

+ 

+         *candidates = subtree_candidates(pb, be, base, e, filter, lookup_returned_allidsp, &err);

+         /* Give the optimised filter back to search filter for free */

+         slapi_pblock_set_filter_executed(pb, filter);

          break;

  

      default:

@@ -1018,20 +1039,17 @@ 

   * Modify the filter to include entries of the referral objectclass

   *

   * make (|(originalfilter)(objectclass=referral))

-  *

-  * "focref, forr" are temporary filters which the caller must free

-  * non-recursively when done with the returned filter.

   */

  static Slapi_Filter *

- create_referral_filter(Slapi_Filter *filter, Slapi_Filter **focref, Slapi_Filter **forr)

+ create_referral_filter(Slapi_Filter *filter)

  {

-     char *buf = slapi_ch_strdup("objectclass=referral");

+     char *buf = slapi_ch_strdup("(objectclass=referral)");

  

-     *focref = slapi_str2filter(buf);

-     *forr = slapi_filter_join(LDAP_FILTER_OR, filter, *focref);

+     Slapi_Filter *focref = slapi_str2filter(buf);

+     Slapi_Filter *forr = slapi_filter_join(LDAP_FILTER_OR, focref, filter);

  

      slapi_ch_free((void **)&buf);

-     return *forr;

+     return forr;

  }

  

  /*

@@ -1039,26 +1057,23 @@ 

   *

   *    (&(parentid=idofbase)(|(originalfilter)(objectclass=referral)))

   *

-  * "fid2kids, focref, fand, forr" are temporary filters which the

-  * caller must free'd non-recursively when done with the returned filter.

-  *

   * This function is exported for the VLV code to use.

   */

  Slapi_Filter *

- create_onelevel_filter(Slapi_Filter *filter, const struct backentry *baseEntry, int managedsait, Slapi_Filter **fid2kids, Slapi_Filter **focref, Slapi_Filter **fand, Slapi_Filter **forr)

+ create_onelevel_filter(Slapi_Filter *filter, const struct backentry *baseEntry, int managedsait)

  {

      Slapi_Filter *ftop = filter;

      char buf[40];

  

      if (!managedsait) {

-         ftop = create_referral_filter(filter, focref, forr);

+         ftop = create_referral_filter(filter);

      }

  

-     sprintf(buf, "parentid=%lu", (u_long)(baseEntry != NULL ? baseEntry->ep_id : 0));

-     *fid2kids = slapi_str2filter(buf);

-     *fand = slapi_filter_join(LDAP_FILTER_AND, ftop, *fid2kids);

+     sprintf(buf, "(parentid=%lu)", (u_long)(baseEntry != NULL ? baseEntry->ep_id : 0));

+     Slapi_Filter *fid2kids = slapi_str2filter(buf);

+     Slapi_Filter *fand = slapi_filter_join(LDAP_FILTER_AND, fid2kids, ftop);

  

-     return *fand;

+     return fand;

  }

  

  /*

@@ -1069,38 +1084,16 @@ 

      Slapi_PBlock *pb,

      backend *be,

      const char *base,

-     struct backentry *e,

      Slapi_Filter *filter,

-     int managedsait,

      int *lookup_returned_allidsp,

      int *err)

  {

-     Slapi_Filter *fid2kids = NULL;

-     Slapi_Filter *focref = NULL;

-     Slapi_Filter *fand = NULL;

-     Slapi_Filter *forr = NULL;

-     Slapi_Filter *ftop = NULL;

      IDList *candidates;

  

-     /*

-      * modify the filter to be something like this:

-      *

-      *    (&(parentid=idofbase)(|(originalfilter)(objectclass=referral)))

-      */

- 

-     ftop = create_onelevel_filter(filter, e, managedsait, &fid2kids, &focref, &fand, &forr);

- 

-     /* from here, it's just like subtree_candidates */

-     candidates = filter_candidates(pb, be, base, ftop, NULL, 0, err);

+     candidates = filter_candidates(pb, be, base, filter, NULL, 0, err);

  

      *lookup_returned_allidsp = slapi_be_is_flag_set(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST);

  

-     /* free up just the filter stuff we allocated above */

-     slapi_filter_free(fid2kids, 0);

-     slapi_filter_free(fand, 0);

-     slapi_filter_free(forr, 0);

-     slapi_filter_free(focref, 0);

- 

      return (candidates);

  }

  

@@ -1115,12 +1108,12 @@ 

   * This function is exported for the VLV code to use.

   */

  Slapi_Filter *

- create_subtree_filter(Slapi_Filter *filter, int managedsait, Slapi_Filter **focref, Slapi_Filter **forr)

+ create_subtree_filter(Slapi_Filter *filter, int managedsait)

  {

      Slapi_Filter *ftop = filter;

  

      if (!managedsait) {

-         ftop = create_referral_filter(filter, focref, forr);

+         ftop = create_referral_filter(filter);

      }

  

      return ftop;

@@ -1137,13 +1130,9 @@ 

      const char *base,

      const struct backentry *e,

      Slapi_Filter *filter,

-     int managedsait,

      int *allids_before_scopingp,

      int *err)

  {

-     Slapi_Filter *focref = NULL;

-     Slapi_Filter *forr = NULL;

-     Slapi_Filter *ftop = NULL;

      IDList *candidates;

      PRBool has_tombstone_filter;

      int isroot = 0;

@@ -1152,17 +1141,22 @@ 

      Operation *op = NULL;

      PRBool is_bulk_import = PR_FALSE;

  

-     /* make (|(originalfilter)(objectclass=referral)) */

-     ftop = create_subtree_filter(filter, managedsait, &focref, &forr);

- 

      /* Fetch a candidate list for the original filter */

-     candidates = filter_candidates_ext(pb, be, base, ftop, NULL, 0, err, allidslimit);

-     slapi_filter_free(forr, 0);

-     slapi_filter_free(focref, 0);

+     candidates = filter_candidates_ext(pb, be, base, filter, NULL, 0, err, allidslimit);

  

      /* set 'allids before scoping' flag */

      if (NULL != allids_before_scopingp) {

-         *allids_before_scopingp = (NULL != candidates && ALLIDS(candidates));

+         /*

+          * If we have candidates, AND they are allids OR we requested the

+          * don't bypass flag: we say that there is all ids etc. So by returning

+          * 1 here, this affects the "can skip filter test", but doing !allids. IE

+          * return 1 here, to FORCE the filter test to be taken!

+          */

+         if (NULL != candidates && (ALLIDS(candidates) || slapi_be_is_flag_set(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST))) {

+             *allids_before_scopingp = 1;

+         } else {

+             *allids_before_scopingp = 0;

+         }

      }

  

      has_tombstone_filter = (filter->f_flags & SLAPI_FILTER_TOMBSTONE);

@@ -1312,10 +1306,12 @@ 

  static int

  can_skip_filter_test(

      Slapi_PBlock *pb __attribute__((unused)),

+     backend *be,

      struct slapi_filter *f,

      int scope,

      IDList *idl)

  {

+     /* If 0, we CAN NOT skip the test, if 1 we can! */

      int rc = 0;

  

      /* Is the ID list ALLIDS ? */

@@ -1335,9 +1331,16 @@ 

      }

  

      /* Grok the filter and tell me if it has only equality components in it */

+     /*

+      * Now that we respect the SLAPI_BE_FLAG in most callers, we may not need this test as it

+      * (correctly) is pretty zealous about testing in most situations.

+      *

+      * Arguably, this removal would just make large set's that are fully indexed faster

+      * to return to the client. But on the flip side, it could let through other issues

+      * so we could consider that we should always filter test.

+      */

      rc = grok_filter(f);

  

- 

      return rc;

  }

  

@@ -1656,8 +1659,6 @@ 

                  /* it's a regular entry, check if it matches the filter, and passes the ACL check */

                  if (0 != (sr->sr_flags & SR_FLAG_CAN_SKIP_FILTER_TEST)) {

                      /* Since we do access control checking in the filter test (?Why?) we need to check access now */

-                     slapi_log_err(SLAPI_LOG_FILTER, "ldbm_back_next_search_entry_ext",

-                                   "Bypassing filter test\n");

                      if (ACL_CHECK_FLAG) {

                          filter_test = slapi_vattr_filter_test_ext(pb, e->ep_entry, filter, ACL_CHECK_FLAG, 1 /* Only perform access checking, thank you */);

                      } else {

@@ -1675,8 +1676,13 @@ 

                                            "Filter bypass ERROR on entry %s\n", backentry_get_ndn(e));

                              filter_test = ft_rc; /* Fix the error */

                          }

+                     } else {

+                         slapi_log_err(SLAPI_LOG_FILTER, "ldbm_back_next_search_entry_ext",

+                                       "Bypassing filter test\n");

                      }

                  } else {

+                     slapi_log_err(SLAPI_LOG_FILTER, "ldbm_back_next_search_entry_ext",

+                                   "Applying filter test\n");

                      /* Old-style case---we need to do a filter test */

                      filter_test = slapi_vattr_filter_test(pb, e->ep_entry, filter, ACL_CHECK_FLAG);

                  }

@@ -571,9 +571,9 @@ 

  /*

   * ldbm_search.c

   */

- Slapi_Filter *create_onelevel_filter(Slapi_Filter *filter, const struct backentry *e, int managedsait, Slapi_Filter **fid2kids, Slapi_Filter **focref, Slapi_Filter **fand, Slapi_Filter **forr);

- Slapi_Filter *create_subtree_filter(Slapi_Filter *filter, int managedsait, Slapi_Filter **focref, Slapi_Filter **forr);

- IDList *subtree_candidates(Slapi_PBlock *pb, backend *be, const char *base, const struct backentry *e, Slapi_Filter *filter, int managedsait, int *allids_before_scopingp, int *err);

+ Slapi_Filter *create_onelevel_filter(Slapi_Filter *filter, const struct backentry *e, int managedsait);

+ Slapi_Filter *create_subtree_filter(Slapi_Filter *filter, int managedsait);

+ IDList *subtree_candidates(Slapi_PBlock *pb, backend *be, const char *base, const struct backentry *e, Slapi_Filter *filter, int *allids_before_scopingp, int *err);

  void search_set_tune(struct ldbminfo *li, int val);

  int search_get_tune(struct ldbminfo *li);

  int compute_lookthrough_limit(Slapi_PBlock *pb, struct ldbminfo *li);

@@ -93,13 +93,13 @@ 

      p->vlv_slapifilter = slapi_str2filter(p->vlv_filter);

      filter_normalize(p->vlv_slapifilter);

      /* make (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) */

-     {

-         Slapi_Filter *fid2kids = NULL;

-         Slapi_Filter *focref = NULL;

-         Slapi_Filter *fand = NULL;

-         Slapi_Filter *forr = NULL;

-         p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, base, 0 /* managedsait */, &fid2kids, &focref, &fand, &forr);

-     }

+     p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, base, 0 /* managedsait */);

+     /*

+      * We lack the information to know if we can optimise - however, given this is

+      * a re-init, we may already be optimised. For now, we be safe, and do not alter

+      * in this context without the ability to check the flags required.

+      */

+     /* slapi_filter_optimise(p->vlv_slapifilter); */

  }

  

  /*

@@ -108,6 +108,7 @@ 

  void

  vlvSearch_init(struct vlvSearch *p, Slapi_PBlock *pb, const Slapi_Entry *e, ldbm_instance *inst)

  {

+     int32_t filter_optimise = 0;

      /* VLV specification */

      /* Need to copy the entry here because this one is in the cache,

       * not forever ! */

@@ -130,6 +131,8 @@ 

          filter_normalize(p->vlv_slapifilter);

      }

  

+     filter_optimise = slapi_pblock_get_filter_optimise_enable(pb);

+ 

      /* JCM: Really should convert the slapifilter into a string and use that. */

  

      /* Convert the filter based on the scope of the search */

@@ -173,22 +176,16 @@ 

  

          /* make (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) */

          {

-             Slapi_Filter *fid2kids = NULL;

-             Slapi_Filter *focref = NULL;

-             Slapi_Filter *fand = NULL;

-             Slapi_Filter *forr = NULL;

-             p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, e, 0 /* managedsait */, &fid2kids, &focref, &fand, &forr);

-             /* jcm: fid2kids, focref, fand, and forr get freed when we free p->vlv_slapifilter */

+             p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, e, 0 /* managedsait */);

+             /* slapi_filter_optimise(p->vlv_slapifilter, filter_optimise); */

              CACHE_RETURN(&inst->inst_cache, &e);

          }

      } break;

      case LDAP_SCOPE_SUBTREE: {

          /* make (|(originalfilter)(objectclass=referral))) */

          /* No need for scope-filter since we apply a scope test before the filter test */

-         Slapi_Filter *focref = NULL;

-         Slapi_Filter *forr = NULL;

-         p->vlv_slapifilter = create_subtree_filter(p->vlv_slapifilter, 0 /* managedsait */, &focref, &forr);

-         /* jcm: focref and forr get freed when we free p->vlv_slapifilter */

+         p->vlv_slapifilter = create_subtree_filter(p->vlv_slapifilter, 0 /* managedsait */);

+         /* slapi_filter_optimise(p->vlv_slapifilter, filter_optimise); */

      } break;

      }

  }

file modified
+13 -2

@@ -1608,7 +1608,8 @@ 

  dse_search(Slapi_PBlock *pb) /* JCM There should only be one exit point from this function! */

  {

      int scope;            /*Scope of the search*/

-     Slapi_Filter *filter; /*The filter*/

+     Slapi_Filter *filter = NULL; /*The filter as we execute it*/

+     Slapi_Filter *filter_original = NULL; /*The original filter*/

      char **attrs;         /*Attributes*/

      int attrsonly;        /*Should we just return the attributes found?*/

      /*int nentries= 0; Number of entries found thus far*/

@@ -1618,6 +1619,7 @@ 

      char returntext[SLAPI_DSE_RETURNTEXT_SIZE] = "";

      Slapi_DN *basesdn = NULL;

      int estimate = 0; /* estimated search result set size */

+     int32_t filter_optimise = 0;

  

      /*

       * Get private information created in the init routine.

@@ -1627,7 +1629,7 @@ 

      if (slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &pdse) < 0 ||

          slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &basesdn) < 0 ||

          slapi_pblock_get(pb, SLAPI_SEARCH_SCOPE, &scope) < 0 ||

-         slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter) < 0 ||

+         slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter_original) < 0 ||

          slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &attrs) < 0 ||

          slapi_pblock_get(pb, SLAPI_SEARCH_ATTRSONLY, &attrsonly) < 0) {

          slapi_send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, NULL, 0, NULL);

@@ -1639,6 +1641,15 @@ 

       */

      isrootdse = slapi_sdn_isempty(basesdn);

  

+     /*

+      * Now optimise the filter for use: we then set it back to the pb

+      * for later free operations.

+      */

+     filter_optimise = slapi_pblock_get_filter_optimise_enable(pb);

+     filter = slapi_filter_dup(filter_original);

+     slapi_filter_optimise(filter, filter_optimise);

+     slapi_pblock_set_filter_executed(pb, filter);

+ 

      switch (scope) {

      case LDAP_SCOPE_BASE: {

          Slapi_Entry *baseentry = NULL;

file modified
+200 -33

@@ -29,7 +29,6 @@ 

  static int get_filter_internal(Connection *conn, BerElement *ber, struct slapi_filter **filt, char **fstr, int maxdepth, int curdepth, int *subentry_dont_rewrite, int *has_tombstone_filter, int *has_ruv_filter);

  static int tombstone_check_filter(Slapi_Filter *f);

  static int ruv_check_filter(Slapi_Filter *f);

- static void filter_optimize(Slapi_Filter *f);

  

  

  /*

@@ -75,7 +74,12 @@ 

                        slapi_filter_to_string(*filt, logbuf, logbufsize));

      }

  

-     filter_optimize(*filt);

+     /*

+      * Filter optimise has been moved to the onelevel/subtree candidate dispatch.

+      * this is because they inject referrals or other business, that we can optimise

+      * and improve.

+      */

+     /* filter_optimize(*filt); */

  

      if (NULL != logbuf) {

          slapi_log_err(SLAPI_LOG_DEBUG, "get_filter", " after optimize: %s\n",

@@ -858,19 +862,22 @@ 

          if (add_to->f_list->f_choice == LDAP_FILTER_NOT) {

              add_this->f_next = add_to->f_list;

              add_to->f_list = add_this;

-             filter_compute_hash(add_to);

-             return_this = add_to;

          } else {

              /* find end of list, add the filter */

              for (fjoin = add_to->f_list; fjoin != NULL; fjoin = fjoin->f_next) {

                  if (fjoin->f_next == NULL) {

                      fjoin->f_next = add_this;

-                     filter_compute_hash(add_to);

-                     return_this = add_to;

                      break;

                  }

              }

          }

+         /*

+          * Make sure we sync the filter flags. The origin filters may have flags