Resolves: bug 249366

Bug Description: rhds71 - search filters returns too many entries on integer attributes value greater than 2 to the power of 31
Reviewed by: nkinder, nhosoi (Thanks!)
Fix Description: The way >= and <= searches are supposed to work in LDAP is that you are supposed to define an ORDERING matching rule for the attribute you want to use in the search filter.  The way our code is written, most strings "just work" as a side effect of the way bdb sorts the keys by default - so you can do (uid>=jvedder) and get what you would expect, even though LDAP says this is illegal because the schema definition of the uid attribute does not have an ORDERING matching rule.  And INTEGER worked with the old binary format for the same reason.  The only attribute definitions we use with ORDERING are attributes that use Generalized Time syntax (e.g. createTimestamp, et. al.) and numSubordinates (which uses INTEGER, but this is a special case handled internally by the db code).
The way it works now is that the indexing code will honor the ORDERING matching rule specified in the schema definition.  Or, if ORDERING is not specified, the user can use the nsMatchingRule index configuration.  This will allow an existing customer that depends all integer syntax attributes (e.g. uidNumber) to allow range searches by default to enable range searches without editing the schema.  The syntax definition for the attribute must also specify a compare function.  This compare function will be used by the bdb bt_compare() function.
I also fixed a bug in the integer normalize code - a string of all zeros should normalize to a single "0".  In all other cases, the leading zeros should be removed.
Platforms tested: RHEL5 x86_64
Flag Day: Yes.  Integer indexes will need to be rebuilt (except for numsubordinates).
Doc impact: Yes - document slapi API additions
QA impact: Pay close attention to tests that use >= or <= search filters, both with and without index attributes.  Also, pay close attention to greater/less than searches using i18n collations.
New Tests integrated into TET: Forthcoming

3 files changed, 44 insertions, 6 deletions

diff --git a/ldap/servers/plugins/syntaxes/cis.c b/ldap/servers/plugins/syntaxes/cis.c
index b3ca4712..20b990dc 100644
--- a/ldap/servers/plugins/syntaxes/cis.c
+++ b/ldap/servers/plugins/syntaxes/cis.c
@@ -84,6 +84,19 @@ static char *boolean_names[] = { "Boolean", BOOLEAN_SYNTAX_OID, 0 };
 static char *time_names[] = { "GeneralizedTime", "time",
 		GENERALIZEDTIME_SYNTAX_OID, 0 };
 
+#define GENERALIZEDTIMEMATCH_OID "2.5.13.27"
+#define GENERALIZEDTIMEORDERINGMATCH_OID "2.5.13.28"
+static Slapi_MatchingRuleEntry
+generalizedTimeMatch = { GENERALIZEDTIMEMATCH_OID, NULL /* no alias? */,
+                         "generalizedTimeMatch", "The rule evaluates to TRUE if and only if the attribute value represents the same universal coordinated time as the assertion value.",
+                         GENERALIZEDTIME_SYNTAX_OID, 0 /* not obsolete */ };
+
+static Slapi_MatchingRuleEntry
+generalizedTimeOrderingMatch = { GENERALIZEDTIMEORDERINGMATCH_OID, NULL /* no alias? */,
+                                 "generalizedTimeOrderingMatch", "The rule evaluates to TRUE if and only if the attribute value represents a universal coordinated time that is earlier than the universal coordinated time represented by the assertion value.",
+                                 GENERALIZEDTIME_SYNTAX_OID, 0 /* not obsolete */ };
+
+
 static char *country_names[] = { "Country String",
 		COUNTRYSTRING_SYNTAX_OID, 0};
 
@@ -223,6 +236,9 @@ time_init( Slapi_PBlock *pb )
 	LDAPDebug( LDAP_DEBUG_PLUGIN, "=> time_init\n", 0, 0, 0 );
 	rc = register_cis_like_plugin( pb, &time_pdesc, time_names,
 			GENERALIZEDTIME_SYNTAX_OID );
+	/* also register this plugin for matching rules */
+	rc |= slapi_matchingrule_register(&generalizedTimeMatch);
+	rc |= slapi_matchingrule_register(&generalizedTimeOrderingMatch);
 	LDAPDebug( LDAP_DEBUG_PLUGIN, "<= time_init %d\n", rc, 0, 0 );
 	return( rc );
 }
diff --git a/ldap/servers/plugins/syntaxes/int.c b/ldap/servers/plugins/syntaxes/int.c
index bca852bb..73c879a7 100644
--- a/ldap/servers/plugins/syntaxes/int.c
+++ b/ldap/servers/plugins/syntaxes/int.c
@@ -58,9 +58,23 @@ static int int_compare(struct berval	*v1, struct berval	*v2);
 /* the first name is the official one from RFC 2252 */
 static char *names[] = { "INTEGER", "int", INTEGER_SYNTAX_OID, 0 };
 
+#define INTEGERMATCH_OID "2.5.13.14"
+#define INTEGERORDERINGMATCH_OID "2.5.13.15"
+
 static Slapi_PluginDesc pdesc = { "int-syntax", PLUGIN_MAGIC_VENDOR_STR,
 	PRODUCTTEXT, "integer attribute syntax plugin" };
 
+static Slapi_MatchingRuleEntry
+integerMatch = { INTEGERMATCH_OID, NULL /* no alias? */,
+                 "integerMatch", "The rule evaluates to TRUE if and only if the attribute value and the assertion value are the same integer value.",
+                 INTEGER_SYNTAX_OID, 0 /* not obsolete */ };
+
+static Slapi_MatchingRuleEntry
+integerOrderingMatch = { INTEGERORDERINGMATCH_OID, NULL /* no alias? */,
+                         "integerOrderingMatch", "The rule evaluates to TRUE if and only if the integer value of the attribute value is less than the integer value of the assertion value.",
+                         INTEGER_SYNTAX_OID, 0 /* not obsolete */ };
+
+
 int
 int_init( Slapi_PBlock *pb )
 {
@@ -88,6 +102,10 @@ int_init( Slapi_PBlock *pb )
 	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_SYNTAX_COMPARE,
 	    (void *) int_compare );
 
+	/* also register this plugin for matching rules */
+	rc |= slapi_matchingrule_register(&integerMatch);
+	rc |= slapi_matchingrule_register(&integerOrderingMatch);
+
 	LDAPDebug( LDAP_DEBUG_PLUGIN, "<= int_init %d\n", rc, 0, 0 );
 	return( rc );
 }
diff --git a/ldap/servers/plugins/syntaxes/value.c b/ldap/servers/plugins/syntaxes/value.c
index 88b3f64a..f654686b 100644
--- a/ldap/servers/plugins/syntaxes/value.c
+++ b/ldap/servers/plugins/syntaxes/value.c
@@ -117,21 +117,25 @@ value_normalize(
 	/* have to do this after trimming spaces */
 	if (syntax & SYNTAX_INT) {
 		int foundsign = 0;
+		int foundzero = 0;
+
 		if (*s == '-') {
 			foundsign = 1;
 			LDAP_UTF8INC(s);
 		}
 
 		while (*s && (*s == '0')) {
+			foundzero = 1;
 			LDAP_UTF8INC(s);
 		}
 
-		/* if there is a hyphen, make sure it is just to the left
-		   of the first significant (i.e. non-zero) digit e.g.
-		   convert -00000001 to -1 */
-		if (foundsign && (s > d)) {
-			*d = '-';
-			d++;
+		if (foundzero && !*s) { /* value is all zeros */
+			*d++ = '0'; /* set value to a single zero */
+		} else if (foundsign && (s > d)) {
+			/* if there is a hyphen, make sure it is just to the left
+			   of the first significant (i.e. non-zero) digit e.g.
+			   convert -00000001 to -1 */
+			*d++ = '-';
 		}
 		/* s should now point at the first significant digit/char */
 	}