r8082: large rewite of ldb_dn.c

- we do not support multpiple attribute components anymore, makes code a lot easier
  they will be readded later if we found out they are really used, so far my tests
  show w2k3 do not handle them as well

- fix escaping issues, move component value to be in an ldb_val structure
  still need to handle binary values case

- make cononicalize functions leak less memory by giving a specific memory context

- fix tests scripts so that test-ldap can start
- make test not delete databases on completion so that I can inspect them
(This used to be commit 624a73148d125690ce18515f19231d26df207738)

5 files changed, 226 insertions, 269 deletions

diff --git a/source4/lib/ldb/common/attrib_handlers.c b/source4/lib/ldb/common/attrib_handlers.c
index 6c5692bcbe..a35a450670 100644
--- a/source4/lib/ldb/common/attrib_handlers.c
+++ b/source4/lib/ldb/common/attrib_handlers.c
@@ -34,10 +34,10 @@
 /*
   default handler that just copies a ldb_val.
 */
-int ldb_handler_copy(struct ldb_context *ldb, 
+int ldb_handler_copy(struct ldb_context *ldb, void *mem_ctx,
 		     const struct ldb_val *in, struct ldb_val *out)
 {
-	*out = ldb_val_dup(ldb, in);
+	*out = ldb_val_dup(mem_ctx, in);
 	if (out->data == NULL) {
 		ldb_oom(ldb);
 		return -1;
@@ -49,11 +49,11 @@ int ldb_handler_copy(struct ldb_context *ldb,
   a case folding copy handler, removing leading and trailing spaces and
   multiple internal spaces
 */
-static int ldb_handler_fold(struct ldb_context *ldb, 
+static int ldb_handler_fold(struct ldb_context *ldb, void *mem_ctx,
 			    const struct ldb_val *in, struct ldb_val *out)
 {
 	uint8_t *s1, *s2;
-	out->data = talloc_size(ldb, strlen(in->data)+1);
+	out->data = talloc_size(mem_ctx, strlen(in->data)+1);
 	if (out->data == NULL) {
 		ldb_oom(ldb);
 		return -1;
@@ -78,20 +78,20 @@ static int ldb_handler_fold(struct ldb_context *ldb,
   a case folding copy handler, removing leading and trailing spaces and
   multiple internal spaces, and checking for wildcard characters
 */
-static int ldb_handler_fold_wildcard(struct ldb_context *ldb, 
+static int ldb_handler_fold_wildcard(struct ldb_context *ldb, void *mem_ctx,
 				     const struct ldb_val *in, struct ldb_val *out)
 {
 	if (strchr(in->data, '*')) {
 		return -1;
 	}
-	return ldb_handler_fold(ldb, in, out);
+	return ldb_handler_fold(ldb, mem_ctx, in, out);
 }
 
 /*
   canonicalise a ldap Integer
   rfc2252 specifies it should be in decimal form
 */
-static int ldb_canonicalise_Integer(struct ldb_context *ldb, 
+static int ldb_canonicalise_Integer(struct ldb_context *ldb, void *mem_ctx,
 				    const struct ldb_val *in, struct ldb_val *out)
 {
 	char *end;
@@ -99,7 +99,7 @@ static int ldb_canonicalise_Integer(struct ldb_context *ldb,
 	if (*end != 0) {
 		return -1;
 	}
-	out->data = talloc_asprintf(ldb, "%lld", i);
+	out->data = talloc_asprintf(mem_ctx, "%lld", i);
 	if (out->data == NULL) {
 		return -1;
 	}
@@ -110,7 +110,7 @@ static int ldb_canonicalise_Integer(struct ldb_context *ldb,
 /*
   compare two Integers
 */
-static int ldb_comparison_Integer(struct ldb_context *ldb, 
+static int ldb_comparison_Integer(struct ldb_context *ldb, void *mem_ctx,
 				  const struct ldb_val *v1, const struct ldb_val *v2)
 {
 	return strtoll(v1->data, NULL, 0) - strtoll(v2->data, NULL, 0);
@@ -119,7 +119,7 @@ static int ldb_comparison_Integer(struct ldb_context *ldb,
 /*
   compare two binary blobs
 */
-int ldb_comparison_binary(struct ldb_context *ldb, 
+int ldb_comparison_binary(struct ldb_context *ldb, void *mem_ctx,
 			  const struct ldb_val *v1, const struct ldb_val *v2)
 {
 	if (v1->length != v2->length) {
@@ -133,7 +133,7 @@ int ldb_comparison_binary(struct ldb_context *ldb,
   and leading and trailing whitespace
   see rfc2252 section 8.1
 */
-static int ldb_comparison_fold(struct ldb_context *ldb, 
+static int ldb_comparison_fold(struct ldb_context *ldb, void *mem_ctx,
 			       const struct ldb_val *v1, const struct ldb_val *v2)
 {
 	const char *s1=v1->data, *s2=v2->data;
@@ -159,7 +159,8 @@ static int ldb_comparison_fold(struct ldb_context *ldb,
   see rfc2252 section 8.1
   handles wildcards
 */
-static int ldb_comparison_fold_wildcard(struct ldb_context *ldb, 
+static int ldb_comparison_fold_wildcard(struct ldb_context *ldb,
+					void *mem_ctx,
 					const struct ldb_val *v1, 
 					const struct ldb_val *v2)
 {
@@ -187,40 +188,49 @@ static int ldb_comparison_fold_wildcard(struct ldb_context *ldb,
 /*
   canonicalise a attribute in DN format
 */
-static int ldb_canonicalise_dn(struct ldb_context *ldb, 
+static int ldb_canonicalise_dn(struct ldb_context *ldb, void *mem_ctx,
 			       const struct ldb_val *in, struct ldb_val *out)
 {
-	struct ldb_dn *dn2=NULL, *dn1 = ldb_dn_explode(ldb, in->data);
+	struct ldb_dn *dn1, *dn2;
+	int ret = -1;
+
+	out->length = 0;
 	out->data = NULL;
+
+	dn1 = ldb_dn_explode(mem_ctx, in->data);
 	if (dn1 == NULL) {
-		goto failed;
+		return -1;
 	}
 	dn2 = ldb_dn_casefold(ldb, dn1);
-	if (dn2 == NULL) goto failed;
+	if (dn2 == NULL) {
+		goto done;
+	}
 
-	out->data = ldb_dn_linearize(ldb, dn2);
-	if (out->data == NULL) goto failed;
+	out->data = ldb_dn_linearize(mem_ctx, dn2);
+	if (out->data == NULL) {
+		goto done;
+	}
+	out->length = strlen(out->data);
 
-	talloc_free(dn1);
-	talloc_free(dn2);
-	return 0;
+	ret = 0;
 
-failed:
+done:
 	talloc_free(dn1);
 	talloc_free(dn2);
-	return -1;
+
+	return ret;
 }
 
 /*
   compare two dns
 */
-static int ldb_comparison_dn(struct ldb_context *ldb, 
+static int ldb_comparison_dn(struct ldb_context *ldb, void *mem_ctx,
 			     const struct ldb_val *v1, const struct ldb_val *v2)
 {
 	struct ldb_val cv1, cv2;
 	int ret;
-	if (ldb_canonicalise_dn(ldb, v1, &cv1) != 0 ||
-	    ldb_canonicalise_dn(ldb, v2, &cv2) != 0) {
+	if (ldb_canonicalise_dn(ldb, mem_ctx, v1, &cv1) != 0 ||
+	    ldb_canonicalise_dn(ldb, mem_ctx, v2, &cv2) != 0) {
 		goto failed;
 	}
 	ret = strcmp(cv1.data, cv2.data);
@@ -236,12 +246,12 @@ failed:
 /*
   compare two objectclasses, looking at subclasses
 */
-static int ldb_comparison_objectclass(struct ldb_context *ldb, 
+static int ldb_comparison_objectclass(struct ldb_context *ldb, void *mem_ctx,
 				      const struct ldb_val *v1, const struct ldb_val *v2)
 {
 	int ret, i;
 	const char **subclasses;
-	ret = ldb_comparison_fold(ldb, v1, v2);
+	ret = ldb_comparison_fold(ldb, mem_ctx, v1, v2);
 	if (ret == 0) {
 		return 0;
 	}
@@ -253,7 +263,7 @@ static int ldb_comparison_objectclass(struct ldb_context *ldb,
 		struct ldb_val vs;
 		vs.data = discard_const(subclasses[i]);
 		vs.length = strlen(subclasses[i]);
-		if (ldb_comparison_objectclass(ldb, &vs, v2) == 0) {
+		if (ldb_comparison_objectclass(ldb, mem_ctx, &vs, v2) == 0) {
 			return 0;
 		}
 	}
diff --git a/source4/lib/ldb/common/ldb_attributes.c b/source4/lib/ldb/common/ldb_attributes.c
index e053ccbbf2..3d4f24771f 100644
--- a/source4/lib/ldb/common/ldb_attributes.c
+++ b/source4/lib/ldb/common/ldb_attributes.c
@@ -60,10 +60,16 @@ int ldb_set_attrib_handlers(struct ldb_context *ldb,
   default function for read/write/canonicalise
 */
 static int ldb_default_copy(struct ldb_context *ldb, 
+			    void *mem_ctx,
 			    const struct ldb_val *in, 
 			    struct ldb_val *out)
 {
-	*out = *in;
+	*out = ldb_val_dup(mem_ctx, in);
+
+	if (out->length == 0) {
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -71,6 +77,7 @@ static int ldb_default_copy(struct ldb_context *ldb,
   default function for comparison
 */
 static int ldb_default_cmp(struct ldb_context *ldb, 
+			    void *mem_ctx,
 			   const struct ldb_val *v1, 
 			   const struct ldb_val *v2)
 {
diff --git a/source4/lib/ldb/common/ldb_dn.c b/source4/lib/ldb/common/ldb_dn.c
index f147197499..18b620b506 100644
--- a/source4/lib/ldb/common/ldb_dn.c
+++ b/source4/lib/ldb/common/ldb_dn.c
@@ -37,63 +37,74 @@
 #include "includes.h"
 #include "ldb/include/ldb.h"
 #include "ldb/include/ldb_private.h"
-#include "ldb/include/ldb_dn.h"
 
 
 #define LDB_DN_NULL_FAILED(x) if (!(x)) goto failed
 
-static char *escape_string(void *mem_ctx, const char *src)
+static char *ldb_dn_escape_value(void *mem_ctx, struct ldb_val value)
 {
-	const char *p, *s;
-	char *d, *dst=NULL;
+	const char *p, *s, *src;
+	char *d, *dst;
+	int len;
 
-	LDB_DN_NULL_FAILED(src);
+	if (!value.length)
+		return NULL;
+
+	p = s = src = (const char *)value.data;
+	len = value.length;
 
 	/* allocate destination string, it will be at most 3 times the source */
-	dst = d = talloc_array(mem_ctx, char, strlen(src) * 3 + 1);
+	dst = d = talloc_array(mem_ctx, char, len * 3 + 1);
 	LDB_DN_NULL_FAILED(dst);
 
-	p = s = src;
+	while (p - src < len) {
 
-	while (*p) {
 		p += strcspn(p, ",=\n+<>#;\\\"");
-		if (*p == '\0') /* no special s found, all ok */
+
+		if (p - src == len) /* found no escapable chars */
 			break;
 
-		if (*p) { /* copy part of the string and escape */
-			memcpy(d, s, p - s);
-			d += (p - s);
+		memcpy(d, s, p - s); /* copy the part of the string before the stop */
+		d += (p - s); /* move to current position */
+
+		if (*p) { /* it is a normal escapable character */
 			*d++ = '\\';
 			*d++ = *p++;
-			s = p;
+		} else { /* we have a zero byte in the string */
+			strncpy(d, "\00", 3); /* escape the zero */
+			d = d + 3;
+			p++; /* skip the zero */
 		}
+		s = p; /* move forward */
 	}
 
 	/* copy the last part (with zero) and return */
-	memcpy(d, s, &src[strlen(src)] - s + 1);
+	memcpy(d, s, &src[len] - s + 1);
 
 	return dst;
+
 failed:
 	talloc_free(dst);
 	return NULL;
 }
 
-static char *unescape_string(void *mem_ctx, const char *src)
+static struct ldb_val ldb_dn_unescape_value(void *mem_ctx, const char *src)
 {
+	struct ldb_val value;
 	unsigned x;
-	char *p, *dst=NULL, *end;
+	char *p, *dst = NULL, *end;
+
+	value.length = 0;
 
 	LDB_DN_NULL_FAILED(src);
 
-	dst = p = talloc_strdup(mem_ctx, src);
+	dst = p = talloc_memdup(mem_ctx, src, strlen(src) + 1);
 	LDB_DN_NULL_FAILED(dst);
 
 	end = &dst[strlen(dst)];
 
 	while (*p) {
 		p += strcspn(p, ",=\n+<>#;\\\"");
-		if (*p == '\0')	/* no escapes or specials found, all ok */
-			return dst;
 
 		if (*p == '\\') {
 			if (strchr(",=\n+<>#;\\\"", p[1])) {
@@ -112,31 +123,41 @@ static char *unescape_string(void *mem_ctx, const char *src)
 			}
 		}
 
-		/* a string with not escaped specials is invalid */	
-
-		return NULL;
+		/* a string with not escaped specials is invalid (tested) */
+		if (*p != '\0') {
+			goto failed;
+		}
 	}
 
-	return dst;
+	value.length = end - dst;
+	value.data = dst;
+	return value;
+
 failed:
 	talloc_free(dst);
-	return NULL;
+	return value;
 }
 
-static char *seek_to_separator(char *string, const char *separator)
-{
-	char *p;
-
-	p = strchr(string, '=');
+/* check if the string contains quotes
+ * skips leading and trailing spaces
+ * - returns 0 if no quotes found
+ * - returns 1 if quotes are found and put their position
+ *   in *quote_start and *quote_end parameters
+ * - return -1 if there are open quotes
+ */
 
-	LDB_DN_NULL_FAILED(p);
+static int get_quotes_position(const char *source, int *quote_start, int *quote_end)
+{
+	const char *p;
 
-	p++;
+	p = source;
 
 	/* check if there are quotes surrounding the value */
-	p += strspn(p, " \n"); /* skip white spaces after '=' */
+	p += strspn(p, " \n"); /* skip white spaces */
 
 	if (*p == '\"') {
+		*quote_start = p - source;
+
 		p++;
 		while (*p != '\"') {
 			p = strchr(p, '\"');
@@ -145,11 +166,50 @@ static char *seek_to_separator(char *string, const char *separator)
 			if (*(p - 1) == '\\')
 				p++;
 		}
+
+		*quote_end = p - source;
+		return 1;
+	}
+
+	return 0;
+
+failed:
+	return -1;
+}
+
+static char *seek_to_separator(char *string, const char *separators)
+{
+	char *p;
+	int ret, qs, qe;
+
+	p = strchr(string, '=');
+	LDB_DN_NULL_FAILED(p);
+
+	p++;
+
+	/* check if there are quotes surrounding the value */
+
+	ret = get_quotes_position(p, &qs, &qe);
+	if (ret == -1)
+		return NULL;
+
+	if (ret == 1) { /* quotes found */
+
+		p += qe; /* positioning after quotes */
+		p += strspn(p, " \n"); /* skip white spaces after the quote */
+
+		if (strcspn(p, separators) != 0) /* if there are characters between quotes */
+			return NULL;	    /* and separators, the dn is invalid */
+
+		return p; /* return on the separator */
 	}
 
-	p += strcspn(p, separator);
+	/* no quotes found seek to separators */
+	ret = strcspn(p, separators);
+	if (ret == 0) /* no separators ?! bail out */
+		return NULL;
 
-	return p;
+	return p + ret;
 
 failed:
 	return NULL;
@@ -172,132 +232,53 @@ static char *ldb_dn_trim_string(char *string, const char *edge)
 	return s;
 }
 
-static struct ldb_dn_attribute *ldb_dn_explode_attribute(void *mem_ctx, char *raw_attribute)
+/* we choosed to not support multpile valued components */
+static struct ldb_dn_component ldb_dn_explode_component(void *mem_ctx, char *raw_component)
 {
-	struct ldb_dn_attribute *at;
+	struct ldb_dn_component dc;
 	char *p;
+	int ret, qs, qe;
 
-	at = talloc(mem_ctx, struct ldb_dn_attribute);
-	LDB_DN_NULL_FAILED(at);
-
-	p = strchr(raw_attribute, '=');
-
+	/* find attribute type/value separator */
+	p = strchr(raw_component, '=');
 	LDB_DN_NULL_FAILED(p);
 
-	*p = '\0';
+	*p++ = '\0'; /* terminate name and point to value */
 
-	at->name = talloc_strdup(at, ldb_dn_trim_string(raw_attribute, " \n"));
-	LDB_DN_NULL_FAILED(at->name);
+	/* copy and trim name in the component */
+	dc.name = talloc_strdup(mem_ctx, ldb_dn_trim_string(raw_component, " \n"));
+	if (!dc.name)
+		return dc;
 
-	p++;
+	ret = get_quotes_position(p, &qs, &qe);
 
-	p = ldb_dn_trim_string(p, " \n");
+	switch (ret) {
+	case 0: /* no quotes trim the string */
+		p = ldb_dn_trim_string(p, " \n");
+		dc.value = ldb_dn_unescape_value(mem_ctx, p);
+		break;
 
-	if (*p == '\"') { /* quotes at start means there must be quotes at the end */
-		if (p[strlen(p) - 1] != '\"') /* malformed value */
-			return NULL;
-	
-		p++;
-		p[strlen(p) - 1] = '\0';
-		at->value = talloc_strdup(at, p);
+	case 1: /* quotes found get the unquoted string */
+		p[qe] = '\0';
+		p = p + qs + 1;
+		dc.value.length = strlen(p);
+		dc.value.data = talloc_memdup(mem_ctx, p, dc.value.length + 1);
+		break;
 
-		return at;
+	default: /* mismatched quotes ot other error, bail out */
+		goto failed;
 	}
-	/* no quotes means we must unescape the string */
-	at->value = unescape_string(at, p);
-	LDB_DN_NULL_FAILED(at->value);
-
-	return at;
-
-failed:
-	talloc_free(at);
-	return NULL;
-}
 
-static struct ldb_dn_component *explode_component(void *mem_ctx, char *raw_component)
-{
-	struct ldb_dn_component *dc;
-	char *p;
-
-	dc = talloc(mem_ctx, struct ldb_dn_component);
-	LDB_DN_NULL_FAILED(dc);
-
-	dc->attr_num = 0;
-	dc->attributes = NULL;
-
-	p = raw_component;
-
-	/* get the components */
-	do {
-		char *t;
-
-		/* terminate the current attribute and return pointer to the next one */
-		t = seek_to_separator(p, "+");
-		LDB_DN_NULL_FAILED(t);
-
-		if (*t) { /* here there is a separator */
-			*t = '\0'; /*terminate */
-			t++; /* a separtor means there's another attribute that follows */
-		}
-
-		/* allocate attributes pointer */
-		dc->attributes = talloc_realloc(dc, dc->attributes,
-						struct ldb_dn_attribute *,
-						dc->attr_num + 1);
-		LDB_DN_NULL_FAILED(dc->attributes);
-
-		/* store the exploded attirbute in the main structure */
-		dc->attributes[dc->attr_num] = ldb_dn_explode_attribute(dc->attributes, p);
-		LDB_DN_NULL_FAILED(dc->attributes[dc->attr_num]);
-
-		dc->attr_num++;
-
-		/* jump to the next attribute if any */
-		p = t;
-
-	} while(*p);
+	if (dc.value.length == 0) {
+		goto failed;
+	}
 
 	return dc;
-failed:
-	talloc_free(dc);
-	return NULL;
-}
-
-/* FIXME: currently consider a dn composed of only case insensitive attributes
-	  this is not correct and need to be fixed soon */
-static void ldb_dn_sort_attributes(struct ldb_dn *edn)
-{
-	struct ldb_dn_attribute *at0, *at1;
-	int i, j, k, l;
 
-	for (i = 0; i < edn->comp_num; i++) {
-		if (edn->components[i]->attr_num > 1) {
-
-			/* it is very unlikely that there is a multivalued RDN. In that
-			   unlikely case it is very unlikely you will find more than 2
-			   values. So the use of bubble sort here seem to be acceptable */
-			for (j = 0; (j + 1) < edn->components[i]->attr_num; j++) {
-				for (k = j; k >= 0; k--) {
-					at0 = edn->components[i]->attributes[k];
-					at1 = edn->components[i]->attributes[k + 1];
-					l = ldb_caseless_cmp(at0->name, at1->name);
-					if (l > 0) {
-						/* already sorted, so no bubbles to move exit inner loop */
-						break;
-					}
-					if (l == 0) {
-						if (ldb_caseless_cmp(at0->value, at1->value) >= 0) {
-							/* already sorted, so no bubbles to move exit inner loop */
-							break;
-						}
-					}
-					
-					edn->components[i]->attributes[k] = at1;
-					edn->components[i]->attributes[k + 1] = at0;
-				}
-			}
-		}
-	}
+failed:
+	talloc_free(dc.name);
+	dc.name = NULL;
+	return dc;
 }
 
 struct ldb_dn *ldb_dn_explode(void *mem_ctx, const char *dn)
@@ -305,6 +286,8 @@ struct ldb_dn *ldb_dn_explode(void *mem_ctx, const char *dn)
 	struct ldb_dn *edn; /* the exploded dn */
 	char *pdn, *p;
 
+	pdn = NULL;
+
 	/* Allocate a structure to hold the exploded DN */
 	edn = talloc(mem_ctx, struct ldb_dn);
 	LDB_DN_NULL_FAILED(edn);
@@ -314,8 +297,7 @@ struct ldb_dn *ldb_dn_explode(void *mem_ctx, const char *dn)
 	edn->components = NULL;
 
 	pdn = p = talloc_strdup(edn, dn);
-	if (!pdn)
-		goto failed;
+	LDB_DN_NULL_FAILED(pdn);
 
 	/* get the components */
 	do {
@@ -323,25 +305,23 @@ struct ldb_dn *ldb_dn_explode(void *mem_ctx, const char *dn)
 
 		/* terminate the current component and return pointer to the next one */
 		t = seek_to_separator(p, ",;");
-		if (t == NULL)
-			goto failed;
+		LDB_DN_NULL_FAILED(t);
 
 		if (*t) { /* here there is a separator */
 			*t = '\0'; /*terminate */
-			t++; /* a separtor means there's another component that follows */
+			t++; /* a separtor means another component follows */
 		}
 
 		/* allocate space to hold the dn component */
 		edn->components = talloc_realloc(edn, edn->components,
-						 struct ldb_dn_component *,
+						 struct ldb_dn_component,
 						 edn->comp_num + 1);
 		if (edn->components == NULL)
 			goto failed;
 
 		/* store the exploded component in the main structure */
-		edn->components[edn->comp_num] = explode_component(edn->components, p);
-		if (edn->components[edn->comp_num] == NULL)
-			goto failed;
+		edn->components[edn->comp_num] = ldb_dn_explode_component(edn, p);
+		LDB_DN_NULL_FAILED(edn->components[edn->comp_num].name);
 
 		edn->comp_num++;
 
@@ -350,87 +330,71 @@ struct ldb_dn *ldb_dn_explode(void *mem_ctx, const char *dn)
 
 	} while(*p);
 
-	/* sort attributes if there's any multivalued component */
-	ldb_dn_sort_attributes(edn);
-
 	talloc_free(pdn);
 	return edn;
 
 failed:
+	talloc_free(pdn);
 	talloc_free(edn);
 	return NULL;
 }
 
-char *ldb_dn_linearize(void *mem_ctx, struct ldb_dn *edn)
+char *ldb_dn_linearize(void *mem_ctx, const struct ldb_dn *edn)
 {
-	char *dn, *ename, *evalue;
-	const char *format;
-	int i, j;
+	char *dn, *value;
+	const char *format = "%s=%s";
+	int i;
 
 	dn = talloc_strdup(mem_ctx, "");
 	LDB_DN_NULL_FAILED(dn);
 
 	for (i = 0; i < edn->comp_num; i++) {
+
 		if (i != 0) {
-			dn = talloc_append_string(mem_ctx, dn, ",");
+			format = ",%s=%s";
 		}
-		for (j = 0; j < edn->components[i]->attr_num; j++) {
-			if (j == 0) {
-				format = "%s=%s";
-			} else {
-				format = "+%s=%s";
-			}
-
-			ename = escape_string(dn, edn->components[i]->attributes[j]->name);
-			LDB_DN_NULL_FAILED(ename);
 
-			evalue = escape_string(dn, edn->components[i]->attributes[j]->value);
-			LDB_DN_NULL_FAILED(evalue);
+		value = ldb_dn_escape_value(dn, edn->components[i].value);
+		LDB_DN_NULL_FAILED(value);
 
-			dn = talloc_asprintf_append(dn, format, ename, evalue);
-			LDB_DN_NULL_FAILED(dn);
+		dn = talloc_asprintf_append(dn, format, edn->components[i].name, value);
+		LDB_DN_NULL_FAILED(dn);
 
-			talloc_free(ename);
-			talloc_free(evalue);
-		}
+		talloc_free(value);
 	}
 
 	return dn;
+
 failed:
 	talloc_free(dn);
 	return NULL;
 }
 
-/* FIXME: currently consider a dn composed of only case insensitive attributes
-	  this is not correct and need to be fixed soon */
-int ldb_dn_compare(struct ldb_dn *edn0, struct ldb_dn *edn1)
+/* compare DNs using casefolding compare functions */
+int ldb_dn_compare(struct ldb_context *ldb, const struct ldb_dn *edn0, const struct ldb_dn *edn1)
 {
-	struct ldb_dn_attribute *at0, *at1;
-	int i, j, k;
+	int i, ret;
 
 	/* if the number of components doesn't match they differ */
 	if (edn0->comp_num != edn1->comp_num)
 		return (edn1->comp_num - edn0->comp_num);
 
 	for (i = 0; i < edn0->comp_num; i++) {
+		const struct ldb_attrib_handler *h;
 
-		/* if the number of attributes per component doesn't match they differ */
-		if (edn0->components[i]->attr_num != edn1->components[i]->attr_num)
-			return (edn1->components[i]->attr_num - edn0->components[i]->attr_num);
-
-		for (j = 0; j < edn0->components[i]->attr_num; j++) {
-			at0 = edn0->components[i]->attributes[j];
-			at1 = edn1->components[i]->attributes[j];
-
-			/* compare names */
-			k = ldb_caseless_cmp(at0->name, at1->name);
-			if (k)
-				return k;
+		/* compare names (attribute names are guaranteed to be ASCII only) */
+		ret = ldb_caseless_cmp(edn0->components[i].name,
+				       edn1->components[i].name);
+		if (ret) {
+			return ret;
+		}
 
-			/* names match, compare values */
-			k = ldb_caseless_cmp(at0->value, at1->value);
-			if (k)
-				return k;
+		/* names match, compare values */
+		h = ldb_attrib_handler(ldb, edn0->components[i].name);
+		ret = h->comparison_fn(ldb, ldb, &(edn0->components[i].value),
+						  &(edn1->components[i].value));
+		if (ret) {
+			return ret;
 		}
 	}
 
@@ -438,55 +402,31 @@ int ldb_dn_compare(struct ldb_dn *edn0, struct ldb_dn *edn1)
 }
 
 /*
-  casefold a dn. We need to uppercase the attribute names, and the 
-  attribute values of case insensitive attributes. We also need to remove
-  extraneous spaces between elements
+  casefold a dn. We need to casefold the attribute names, and canonicalize 
+  attribute values of case insensitive attributes.
 */
-struct ldb_dn *ldb_dn_casefold(struct ldb_context *ldb, struct ldb_dn *edn)
+struct ldb_dn *ldb_dn_casefold(struct ldb_context *ldb, const struct ldb_dn *edn)
 {
 	struct ldb_dn *cedn;
-	int i, j;
+	int i;
 
 	cedn = talloc(ldb, struct ldb_dn);
 	LDB_DN_NULL_FAILED(cedn);
 
 	cedn->comp_num = edn->comp_num;
-	cedn->components = talloc_array(cedn, struct ldb_dn_component *, edn->comp_num);
+	cedn->components = talloc_array(cedn, struct ldb_dn_component, edn->comp_num);
 	LDB_DN_NULL_FAILED(cedn->components);
 
 	for (i = 0; i < edn->comp_num; i++) {
-		struct ldb_dn_component *dc;
+		struct ldb_dn_component dc;
+		const struct ldb_attrib_handler *h;
 
-		dc = talloc(cedn->components, struct ldb_dn_component);
-		LDB_DN_NULL_FAILED(dc);
+		dc.name = ldb_casefold(cedn, edn->components[i].name);
+		LDB_DN_NULL_FAILED(dc.name);
 
-		dc->attr_num = edn->components[i]->attr_num;
-		dc->attributes = edn->components[i]->attributes;
-		LDB_DN_NULL_FAILED(dc->attributes);
-
-		for (j = 0; j < edn->components[i]->attr_num; j++) {
-			struct ldb_dn_attribute *at;
-			struct ldb_val v0, v;
-			const struct ldb_attrib_handler *h;
-
-			at = talloc(dc->attributes, struct ldb_dn_attribute);
-			LDB_DN_NULL_FAILED(at);
-
-			at->name = ldb_casefold(at, edn->components[i]->attributes[j]->name);
-			LDB_DN_NULL_FAILED(at->name);
-
-			h = ldb_attrib_handler(ldb, at->name);
-			/* at->value should be a ldb_val, work around
-			   this for now .... */
-			v0.data = edn->components[i]->attributes[j]->value;
-			v0.length = strlen(v0.data);
-			if (h->canonicalise_fn(ldb, &v0, &v) != 0) {
-				return NULL;
-			}
-
-			talloc_steal(at, v.data);
-			at->value = v.data;
-			dc->attributes[j] = at;
+		h = ldb_attrib_handler(ldb, dc.name);
+		if (h->canonicalise_fn(ldb, cedn, &(edn->components[i].value), &(dc.value)) != 0) {
+			goto failed;
 		}
 
 		cedn->components[i] = dc;
diff --git a/source4/lib/ldb/common/ldb_ldif.c b/source4/lib/ldb/common/ldb_ldif.c
index deeb84b3c0..79ec857cbd 100644
--- a/source4/lib/ldb/common/ldb_ldif.c
+++ b/source4/lib/ldb/common/ldb_ldif.c
@@ -317,7 +317,7 @@ int ldb_ldif_write(struct ldb_context *ldb,
 
 		for (j=0;j<msg->elements[i].num_values;j++) {
 			struct ldb_val v;
-			ret = h->ldif_write_fn(ldb, &msg->elements[i].values[j], &v);
+			ret = h->ldif_write_fn(ldb, ldb, &msg->elements[i].values[j], &v);
 			CHECK_RET;
 			if (ldb_should_b64_encode(&v)) {
 				ret = fprintf_fn(private_data, "%s:: ", 
@@ -647,7 +647,7 @@ struct ldb_ldif *ldb_ldif_read(struct ldb_context *ldb,
 			if (!el->values) {
 				goto failed;
 			}
-			ret = h->ldif_read_fn(ldb, &value, &el->values[el->num_values]);
+			ret = h->ldif_read_fn(ldb, ldif, &value, &el->values[el->num_values]);
 			if (ret != 0) {
 				goto failed;
 			}
@@ -671,7 +671,7 @@ struct ldb_ldif *ldb_ldif_read(struct ldb_context *ldb,
 				goto failed;
 			}
 			el->num_values = 1;
-			ret = h->ldif_read_fn(ldb, &value, &el->values[0]);
+			ret = h->ldif_read_fn(ldb, ldif, &value, &el->values[0]);
 			if (ret != 0) {
 				goto failed;
 			}
diff --git a/source4/lib/ldb/common/ldb_match.c b/source4/lib/ldb/common/ldb_match.c
index 45a482066e..462c078d81 100644
--- a/source4/lib/ldb/common/ldb_match.c
+++ b/source4/lib/ldb/common/ldb_match.c
@@ -120,7 +120,7 @@ static int ldb_match_leaf(struct ldb_context *ldb,
 	h = ldb_attrib_handler(ldb, el->name);
 
 	for (i=0;i<el->num_values;i++) {
-		if (h->comparison_fn(ldb, &tree->u.simple.value, 
+		if (h->comparison_fn(ldb, ldb, &tree->u.simple.value, 
 				     &el->values[i]) == 0) {
 			return 1;
 		}