[PATCH] ldb_debug: Fix for bug 13185

[Douglas Bagnall]

On 22/02/18 16:06, Gary Lockyer via samba-technical wrote:
> +	/*
> +	 * Check for duplicates in unique indexes
>  	 */
>  	if (list->count > 0 &&
>  	    ((a != NULL
>  	      && (a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX ||
> -		 (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))) ||
> -	     ldb_attr_cmp(el->name, LTDB_IDXDN) == 0)) {
> +		  (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))))) {

This looks a bit wrong. I don't think it actually is wrong, but it
just looks scary having "&" mixing with other operators without tight
parentheses. I had to read random web pages to be sure the precedence
of (a & b || c) works as intended.

I think something like this is logically the same but a bit clearer:

 	if (list->count > 0 &&
            a != NULL &&
 	    ((a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX) ||
             (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))) {

as per the attached patch.

Douglas
-------------- next part --------------
From d2f309c96c1f31773e6cfd1115af7313076cb953 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Thu, 22 Feb 2018 15:52:15 +1300
Subject: [PATCH] ldb_debug: Fix binary data in debug log

When duplicate objects were added, the GUID was printed in the debug log
The GUID was not escaped and therefore displayed as binary content.

This patch splits out the duplicate DN creation error and the duplicate
GIUD error.  Duplicate DN's are a normal event and don't require debug
logging.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13185

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
---
 lib/ldb/ldb_tdb/ldb_index.c | 69 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 54 insertions(+), 15 deletions(-)

diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index f2fce42eac7..955acf69aa4 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -1820,28 +1820,67 @@ static int ltdb_index_add1(struct ldb_module *module,
 	}
 
 	/*
-	 * Check for duplicates in unique indexes and for the @IDXDN
-	 * DN -> GUID record
+	 * Check for duplicates in the @IDXDN DN -> GUID record
+	 *
+	 * This is very normal, it just means a duplicate DN creation
+	 * was attempted, so don't set the error string or print scary
+	 * messages.
 	 */
 	if (list->count > 0 &&
-	    ((a != NULL
-	      && (a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX ||
-		 (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))) ||
-	     ldb_attr_cmp(el->name, LTDB_IDXDN) == 0)) {
+	    ldb_attr_cmp(el->name, LTDB_IDXDN) == 0) {
+		talloc_free(list);
+		return LDB_ERR_CONSTRAINT_VIOLATION;
+	}
+
+	/*
+	 * Check for duplicates in unique indexes
+	 */
+	if (list->count > 0 &&
+	    a != NULL &&
+	    ((a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX) ||
+	     (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))) {
 		/*
 		 * We do not want to print info about a possibly
 		 * confidential DN that the conflict was with in the
 		 * user-visible error string
 		 */
-		ldb_debug(ldb, LDB_DEBUG_WARNING,
-			  __location__ ": unique index violation on %s in %s, "
-			  "conficts with %*.*s in %s",
-			  el->name, ldb_dn_get_linearized(msg->dn),
-			  (int)list->dn[0].length,
-			  (int)list->dn[0].length,
-			  list->dn[0].data,
-			  ldb_dn_get_linearized(dn_key));
-		ldb_asprintf_errstring(ldb, __location__ ": unique index violation on %s in %s",
+
+		if (ltdb->cache->GUID_index_attribute == NULL) {
+			ldb_debug(ldb, LDB_DEBUG_WARNING,
+				  __location__
+				  ": unique index violation on %s in %s, "
+				  "conficts with %*.*s in %s",
+				  el->name, ldb_dn_get_linearized(msg->dn),
+				  (int)list->dn[0].length,
+				  (int)list->dn[0].length,
+				  list->dn[0].data,
+				  ldb_dn_get_linearized(dn_key));
+		} else {
+			/* This can't fail, gives a default at worst */
+			const struct ldb_schema_attribute *attr
+				= ldb_schema_attribute_by_name(
+					ldb,
+					ltdb->cache->GUID_index_attribute);
+			struct ldb_val v;
+			ret = attr->syntax->ldif_write_fn(ldb, list,
+							  &list->dn[0], &v);
+			if (ret == LDB_SUCCESS) {
+				ldb_debug(ldb, LDB_DEBUG_WARNING,
+					  __location__
+					  ": unique index violation on %s in "
+					  "%s, conficts with %s %*.*s in %s",
+					  el->name,
+					  ldb_dn_get_linearized(msg->dn),
+					  ltdb->cache->GUID_index_attribute,
+					  (int)v.length,
+					  (int)v.length,
+					  v.data,
+					  ldb_dn_get_linearized(dn_key));
+			}
+		}
+		ldb_asprintf_errstring(ldb,
+				       __location__ ": unique index violation "
+				       "on %s in %s",
 				       el->name,
 				       ldb_dn_get_linearized(msg->dn));
 		talloc_free(list);
-- 
2.14.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 500 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180222/ef5d25dc/signature.sig>