Resolves: bug 249366

Bug Description: rhds71 - search filters returns too many entries on interger attributes value greater than 2 to the 31
Reviewed by: nkinder, nhosoi (Thanks!)
Fix Description: I found a bug in my previous patch.  The bt_compare function is used not only for comparing the actual key values but also for comparing raw index keys - that is, keys with the leading '=' or '*'.  If comparing two keys, we should only use the syntax specific compare function if we are comparing two valid equality keys.  A valid equality key begins with EQ_PREFIX and has at least one character after that.  In this case, we strip off the EQ_PREFIX and pass the values to the syntax specific compare function.  Otherwise, we just use a simple berval compare function that is based on memcmp.
The code in index_range_read needs to use a similar comparison algorithm, so I beefed up DBTcmp.
Why is this necessary?  When doing a >= search or a <= search, we need to get the upper (for >=) or lower (for <=) bound for the range, which will either be the last (for >=) or first (for <=) equality key in the index.  The index code uses a key of '=' to find the lower bound (which is lower than any key "=value") and a key of '>' to find the upper bound.  A '=' with no value will collate before any real eq key with a value, and the ascii value of '>' is one greater than the ascii value of '='.
Platforms tested: RHEL5 x86_64
Flag Day: no
Doc impact: no
QA impact: should be covered by regular nightly and manual testing
New Tests integrated into TET: none

2 files changed, 68 insertions, 14 deletions

diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c
index 141662c2..68ea3338 100644
--- a/ldap/servers/slapd/back-ldbm/dblayer.c
+++ b/ldap/servers/slapd/back-ldbm/dblayer.c
@@ -232,22 +232,42 @@ static int dblayer_db_remove_ex(dblayer_private_env *env, char const path[], cha
    always normalize both arguments.  We need to add an additional
    syntax compare function that does not normalize or takes
    an argument like value_cmp to specify to normalize or not.
-*/
 
-typedef int (*syntax_cmp_fn_type)(struct berval *, struct berval *);
+   More fun - this function is used to compare both raw database
+   keys (e.g. with the prefix '=' or '+' or '*' etc.) and without
+   (in the case of two equality keys, we want to strip off the
+   leading '=' to compare the actual values).  We only use the
+   value_compare function if both keys are equality keys with
+   some data after the equality prefix.  In every other case,
+   we will just use a standard berval cmp function.
+
+   see also DBTcmp
+*/
 static int
 dblayer_bt_compare(DB *db, const DBT *dbt1, const DBT *dbt2)
 {
     struct berval bv1, bv2;
     value_compare_fn_type syntax_cmp_fn = (value_compare_fn_type)db->app_private;
 
-    bv1.bv_val = (char *)dbt1->data+1; /* remove leading '=' */
-    bv1.bv_len = (ber_len_t)dbt1->size-1;
+    if ((dbt1->data && (dbt1->size>1) && (*((char*)dbt1->data) == EQ_PREFIX)) &&
+        (dbt2->data && (dbt2->size>1) && (*((char*)dbt2->data) == EQ_PREFIX))) {
+        bv1.bv_val = (char *)dbt1->data+1; /* remove leading '=' */
+        bv1.bv_len = (ber_len_t)dbt1->size-1;
+
+        bv2.bv_val = (char *)dbt2->data+1; /* remove leading '=' */
+        bv2.bv_len = (ber_len_t)dbt2->size-1;
+
+        return syntax_cmp_fn(&bv1, &bv2);
+    }
+
+    /* else compare two "raw" index keys */
+    bv1.bv_val = (char *)dbt1->data;
+    bv1.bv_len = (ber_len_t)dbt1->size;
 
-    bv2.bv_val = (char *)dbt2->data+1; /* remove leading '=' */
-    bv2.bv_len = (ber_len_t)dbt2->size-1;
+    bv2.bv_val = (char *)dbt2->data;
+    bv2.bv_len = (ber_len_t)dbt2->size;
 
-    return syntax_cmp_fn(&bv1, &bv2);
+    return slapi_berval_cmp(&bv1, &bv2);
 }
 
 /* this flag use if user remotely turned batching off */
diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c
index 9da2389c..344f33f1 100644
--- a/ldap/servers/slapd/back-ldbm/index.c
+++ b/ldap/servers/slapd/back-ldbm/index.c
@@ -895,16 +895,50 @@ index_read_ext(
 	return( idl );
 }
 
+/* This function compares two index keys.  It is assumed
+   that the values are already normalized, since they should have
+   been when the index was created (by int_values2keys).
+
+   richm - actually, the current syntax compare functions
+   always normalize both arguments.  We need to add an additional
+   syntax compare function that does not normalize or takes
+   an argument like value_cmp to specify to normalize or not.
+
+   More fun - this function is used to compare both raw database
+   keys (e.g. with the prefix '=' or '+' or '*' etc.) and without
+   (in the case of two equality keys, we want to strip off the
+   leading '=' to compare the actual values).  We only use the
+   value_compare function if both keys are equality keys with
+   some data after the equality prefix.  In every other case,
+   we will just use a standard berval cmp function.
+
+   see also dblayer_bt_compare
+*/
 static int
-DBTcmp (DBT* L, DBT* R)
+DBTcmp (DBT* L, DBT* R, value_compare_fn_type cmp_fn)
 {
     struct berval Lv;
     struct berval Rv;
-    Lv.bv_val = L->dptr; Lv.bv_len = L->dsize;
-    Rv.bv_val = R->dptr; Rv.bv_len = R->dsize;
-    return slapi_berval_cmp (&Lv, &Rv);
+
+    if ((L->data && (L->size>1) && (*((char*)L->data) == EQ_PREFIX)) &&
+        (R->data && (R->size>1) && (*((char*)R->data) == EQ_PREFIX))) {
+        Lv.bv_val = (char*)L->data+1; Lv.bv_len = (ber_len_t)L->size-1;
+        Rv.bv_val = (char*)R->data+1; Rv.bv_len = (ber_len_t)R->size-1;
+        /* use specific compare fn, if any */
+        cmp_fn = (cmp_fn ? cmp_fn : slapi_berval_cmp);
+    } else {
+        Lv.bv_val = (char*)L->data; Lv.bv_len = (ber_len_t)L->size;
+        Rv.bv_val = (char*)R->data; Rv.bv_len = (ber_len_t)R->size;
+        /* just compare raw bervals */
+        cmp_fn = slapi_berval_cmp;
+    }
+    return cmp_fn(&Lv, &Rv);
 }
 
+/* This only works with normalized keys, which
+   should be ok because at this point both L and R
+   should have already been normalized
+*/
 #define DBT_EQ(L,R) ((L)->dsize == (R)->dsize &&\
  ! memcmp ((L)->dptr, (R)->dptr, (L)->dsize))
 
@@ -1145,7 +1179,7 @@ index_range_read(
                         "index_range_read(%s,%s) seek to end of index file err %i\n",
                         type, prefix, *err );
                 }
-            } else if (DBTcmp (&upperkey, &cur_key) > 0) {
+            } else if (DBTcmp (&upperkey, &cur_key, ai->ai_key_cmp_fn) > 0) {
                 tmpbuf = slapi_ch_realloc (tmpbuf, cur_key.dsize);
                 memcpy (tmpbuf, cur_key.dptr, cur_key.dsize);
                 DBT_FREE_PAYLOAD(upperkey);
@@ -1233,8 +1267,8 @@ index_range_read(
     }
     while (*err == 0 &&
            (operator == SLAPI_OP_LESS) ?
-           DBTcmp(&cur_key, &upperkey) < 0 :
-           DBTcmp(&cur_key, &upperkey) <= 0) {
+           DBTcmp(&cur_key, &upperkey, ai->ai_key_cmp_fn) < 0 :
+           DBTcmp(&cur_key, &upperkey, ai->ai_key_cmp_fn) <= 0) {
         /* exit the loop when we either run off the end of the table,
          * fail to read a key, or read a key that's out of range.
          */