Bläddra i källkod

Ticket 49432 - filter optimise crash

Bug Description:  In a certain condition with a filter, when we
removed the equality candidate to optimise it, with a nested
and, during the merge process we would segfault

Fix Description:  Fix the merge subfilter process to be cleaner
and work in all conditions. Merge the set of filter tests to
cmocka in addition to the python tests to help catch this earlier

https://pagure.io/389-ds-base/issue/49432

Author: wibrown

Review by: mreynolds (Thanks!)
William Brown 8 år sedan
förälder
incheckning
5c89dd8f9c
5 ändrade filer med 106 tillägg och 12 borttagningar
  1. 1 0
      Makefile.am
  2. 18 12
      ldap/servers/slapd/filter.c
  3. 83 0
      test/libslapd/filter/optimise.c
  4. 1 0
      test/libslapd/test.c
  5. 3 0
      test/test_slapd.h

+ 1 - 0
Makefile.am

@@ -1994,6 +1994,7 @@ TESTS = test_slapd \
 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 \

+ 18 - 12
ldap/servers/slapd/filter.c

@@ -1555,22 +1555,28 @@ filter_prioritise_element(Slapi_Filter **list, Slapi_Filter **head, Slapi_Filter
 
 static void
 filter_merge_subfilter(Slapi_Filter **list, Slapi_Filter **f_prev, Slapi_Filter **f_cur, Slapi_Filter **f_next)   {
-    /* Cut our current AND/OR out */
-    if (*f_prev != NULL) {
-        (*f_prev)->f_next = (*f_cur)->f_next;
-    } else if (*list == *f_cur) {
-        *list = (*f_cur)->f_next;
-    }
-    (*f_next) = (*f_cur)->f_next;
 
-    /* Look ahead to the end of our list, without the f_cur. */
-    Slapi_Filter *f_cur_tail = *list;
+    /* First, graft in the new item between f_cur and f_cur -> f_next */
+    Slapi_Filter *remainder = (*f_cur)->f_next;
+    (*f_cur)->f_next = (*f_cur)->f_list;
+    /* Go to the end of the newly grafted list, and put in our remainder. */
+    Slapi_Filter *f_cur_tail = *f_cur;
     while (f_cur_tail->f_next != NULL) {
         f_cur_tail = f_cur_tail->f_next;
     }
-    /* Now append our descendant into the tail */
-    f_cur_tail->f_next = (*f_cur)->f_list;
-    /* Finally free the remainder */
+    f_cur_tail->f_next = remainder;
+
+    /* Now indicate to the caller what the next element is. */
+    *f_next = (*f_cur)->f_next;
+
+    /* Now that we have grafted our list in, cut out f_cur */
+    if (*f_prev != NULL) {
+        (*f_prev)->f_next = *f_next;
+    } else if (*list == *f_cur) {
+        *list = *f_next;
+    }
+
+    /* Finally free the f_cur (and/or) */
     slapi_filter_free(*f_cur, 0);
 }
 

+ 83 - 0
test/libslapd/filter/optimise.c

@@ -0,0 +1,83 @@
+/** BEGIN COPYRIGHT BLOCK
+ * Copyright (C) 2017 Red Hat, Inc.
+ * All rights reserved.
+ *
+ * License: GPL (version 3 or any later version).
+ * See LICENSE for details.
+ * END COPYRIGHT BLOCK **/
+
+#include "../../test_slapd.h"
+
+/* To access filter optimise */
+#include <slapi-private.h>
+
+void
+test_libslapd_filter_optimise(void **state __attribute__((unused)))
+{
+    char *test_filters[] = {
+        "(&(uid=uid1)(sn=last1)(givenname=first1))",
+        "(&(uid=uid1)(&(sn=last1)(givenname=first1)))",
+        "(&(uid=uid1)(&(&(sn=last1))(&(givenname=first1))))",
+        "(&(uid=*)(sn=last3)(givenname=*))",
+        "(&(uid=*)(&(sn=last3)(givenname=*)))",
+        "(&(uid=uid5)(&(&(sn=*))(&(givenname=*))))",
+        "(&(objectclass=*)(uid=*)(sn=last*))",
+        "(&(objectclass=*)(uid=*)(sn=last1))",
+
+        "(|(uid=uid1)(sn=last1)(givenname=first1))",
+        "(|(uid=uid1)(|(sn=last1)(givenname=first1)))",
+        "(|(uid=uid1)(|(|(sn=last1))(|(givenname=first1))))",
+        "(|(objectclass=*)(sn=last1)(|(givenname=first1)))",
+        "(|(&(objectclass=*)(sn=last1))(|(givenname=first1)))",
+        "(|(&(objectclass=*)(sn=last))(|(givenname=first1)))",
+
+        "(&(uid=uid1)(!(cn=NULL)))",
+        "(&(!(cn=NULL))(uid=uid1))",
+        "(&(uid=*)(&(!(uid=1))(!(givenname=first1))))",
+
+        "(&(|(uid=uid1)(uid=NULL))(sn=last1))",
+        "(&(|(uid=uid1)(uid=NULL))(!(sn=NULL)))",
+        "(&(|(uid=uid1)(sn=last2))(givenname=first1))",
+        "(|(&(uid=uid1)(!(uid=NULL)))(sn=last2))",
+        "(|(&(uid=uid1)(uid=NULL))(sn=last2))",
+        "(&(uid=uid5)(sn=*)(cn=*)(givenname=*)(uid=u*)(sn=la*)(cn=full*)(givenname=f*)(uid>=u)(!(givenname=NULL)))",
+        "(|(&(objectclass=*)(sn=last))(&(givenname=first1)))",
+
+        "(&(uid=uid1)(sn=last1)(givenname=NULL))",
+        "(&(uid=uid1)(&(sn=last1)(givenname=NULL)))",
+        "(&(uid=uid1)(&(&(sn=last1))(&(givenname=NULL))))",
+        "(&(uid=uid1)(&(&(sn=last1))(&(givenname=NULL)(sn=*)))(|(sn=NULL)))",
+        "(&(uid=uid1)(&(&(sn=last*))(&(givenname=first*)))(&(sn=NULL)))",
+
+        "(|(uid=NULL)(sn=NULL)(givenname=NULL))",
+        "(|(uid=NULL)(|(sn=NULL)(givenname=NULL)))",
+        "(|(uid=NULL)(|(|(sn=NULL))(|(givenname=NULL))))",
+
+        "(uid>=uid3)",
+        "(&(uid=*)(uid>=uid3))",
+        "(|(uid>=uid3)(uid<=uid5))",
+        "(&(uid>=uid3)(uid<=uid5))",
+        "(|(&(uid>=uid3)(uid<=uid5))(uid=*))",
+
+        "(|(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)"
+        "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)"
+        "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)"
+        "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)"
+        "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)"
+        "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)"
+        "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)"
+        "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)"
+        "(uid=*))",
+        NULL
+    };
+
+    for (size_t i = 0; test_filters[i] != NULL; i++) {
+        char *filter_str = slapi_ch_strdup(test_filters[i]);
+
+        struct slapi_filter *filter = slapi_str2filter(filter_str);
+        slapi_filter_optimise(filter);
+        slapi_filter_free(filter, 1);
+        slapi_ch_free_string(&filter_str);
+    }
+}
+

+ 1 - 0
test/libslapd/test.c

@@ -28,6 +28,7 @@ run_libslapd_tests(void)
         cmocka_unit_test(test_libslapd_operation_v3c_target_spec),
         cmocka_unit_test(test_libslapd_counters_atomic_usage),
         cmocka_unit_test(test_libslapd_counters_atomic_overflow),
+        cmocka_unit_test(test_libslapd_filter_optimise),
         cmocka_unit_test(test_libslapd_pal_meminfo),
         cmocka_unit_test(test_libslapd_util_cachesane),
     };

+ 3 - 0
test/test_slapd.h

@@ -26,6 +26,9 @@ int run_plugin_tests(void);
 /* libslapd */
 void test_libslapd_hello(void **state);
 
+/* libslapd-filter-optimise */
+void test_libslapd_filter_optimise(void **state);
+
 /* libslapd-pblock-analytics */
 void test_libslapd_pblock_analytics(void **state);