Re: (ITS#5540) Normalization assertion in attr.c

[hyc@symas.com]

This is a multi-part message in MIME format.
--------------050600010007030200030106
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

hyc@symas.com wrote:
> We'll probably need to discuss on the -devel list what steps to take from here.
>
I wrote the attached patch for the original issue. Unfortunately it asserted 
right away:

[Switching to Thread 1090525504 (LWP 23577)]
0x00002b55e738d535 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x00002b55e738d535 in raise () from /lib64/libc.so.6
#1  0x00002b55e738e990 in abort () from /lib64/libc.so.6
#2  0x00002b55e7386c16 in __assert_fail () from /lib64/libc.so.6
#3  0x00000000004410e5 in attr_valadd (a=0xb866c8, vals=0x41000670, nvals=0x0, 
nn=1) at ../../../r24/servers/slapd/attr.c:394
#4  0x0000000000441a06 in attr_merge_one (e=0xb72c08, desc=0x8b3780, 
val=0x41000670, nval=0x0)
     at ../../../r24/servers/slapd/attr.c:599
#5  0x000000000043ea0f in slap_add_opattrs (op=0x965ec0, text=<value optimized 
out>, textbuf=<value optimized out>,
     textlen=<value optimized out>, manage_ctxcsn=<value optimized out>) at 
../../../r24/servers/slapd/add.c:664
#6  0x00000000004d9c42 in hdb_add (op=0x965ec0, rs=0x41000ca0) at add.c:108
#7  0x000000000043f1a6 in fe_op_add (op=0x965ec0, rs=0x41000ca0) at 
../../../r24/servers/slapd/add.c:334
#8  0x000000000043fa12 in do_add (op=0x965ec0, rs=0x41000ca0) at 
../../../r24/servers/slapd/add.c:194
#9  0x00000000004383a7 in connection_operation (ctx=0x41000de0, 
arg_v=0x965ec0) at ../../../r24/servers/slapd/connection.c:1084
#10 0x00000000004388cf in connection_read_thread (ctx=0x41000de0, argv=0xc) at 
../../../r24/servers/slapd/connection.c:1211
#11 0x000000000054b8ca in ldap_int_thread_pool_wrapper (xpool=0x8bf070) at 
../../../r24/libraries/libldap_r/tpool.c:663
#12 0x00002b55e655e09e in start_thread () from /lib64/libpthread.so.0
#13 0x00002b55e741e4cd in clone () from /lib64/libc.so.6
#14 0x0000000000000000 in ?? ()

It was adding the createTimestamp attribute, without providing normalized 
values. slap_add_opattrs was written before the generalizedTimeNormalize 
function was written... I suspect there will be a fair number of cases that 
need to be cleaned up. I don't have time at the moment to chase them all down. 
Anyone else want to jump in here? If not, we may have to push this back a bit. 
Note that this patch will probably require a fair number of databases to be 
reloaded.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

--------------050600010007030200030106
Content-Type: text/plain;
 name="dif.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="dif.txt"

Index: attr.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/attr.c,v
retrieving revision 1.112.2.7
diff -u -r1.112.2.7 attr.c
--- attr.c	11 Feb 2008 23:26:43 -0000	1.112.2.7
+++ attr.c	30 May 2008 09:33:43 -0000
@@ -366,8 +366,39 @@
 	BerVarray nvals,
 	int nn )
 {
-	int		i;
 	BerVarray	v2;
+	int		i;
+
+	{
+		/* FIXME: if the schema has been edited, and an equality matching rule
+		 * has been added or removed from the attribute definition, the database
+		 * values may no longer be in sync with our expectations. Currently this
+		 * means the DB must be reloaded.
+		 */
+		int have_norm, have_nval, new_nval;
+		have_norm = a->a_desc->ad_type->sat_equality->smr_normalize != NULL;
+		have_nval = a->a_nvals != a->a_vals;
+		new_nval = nvals != NULL;
+
+		/* this check is only relevant if any values already exist */
+		if ( a->a_vals != NULL && have_norm != have_nval ) {
+			Debug(LDAP_DEBUG_ANY,
+				"attr_valadd: database inconsistent with schema definition of %s, reload the DB\n",
+					a->a_desc->ad_cname.bv_val, 0, 0 );
+			return LDAP_OTHER;
+			/* no need to compare have_nval with new_nval; by transitivity they will match if
+			 * the above check and the following check succeed.
+			 */
+		}
+
+		assert( have_norm == new_nval );
+		if ( have_norm != new_nval ) {
+			Debug(LDAP_DEBUG_ANY,
+				"attr_valadd: new values of %s not properly normalized (should never happen!)\n",
+					a->a_desc->ad_cname.bv_val, 0, 0 );
+			return LDAP_OTHER;
+		}
+	}
 
 	v2 = (BerVarray) SLAP_REALLOC( (char *) a->a_vals,
 		    (a->a_numvals + nn + 1) * sizeof(struct berval) );
@@ -469,15 +500,6 @@
 
 	if ( *a == NULL ) {
 		*a = attr_alloc( desc );
-	} else {
-		/*
-		 * FIXME: if the attribute already exists, the presence
-		 * of nvals and the value of (*a)->a_nvals must be consistent
-		 */
-		assert( ( nvals == NULL && (*a)->a_nvals == (*a)->a_vals )
-				|| ( nvals != NULL && (
-					( (*a)->a_vals == NULL && (*a)->a_nvals == NULL )
-					|| ( (*a)->a_nvals != (*a)->a_vals ) ) ) );
 	}
 
 	if ( vals != NULL ) {

--------------050600010007030200030106--