[SCM] Samba Shared Repository - branch master updated

Matthias Dieter Wallnöfer mdw at samba.org
Wed Oct 7 16:19:00 MDT 2009


The branch, master has been updated
       via  91456b8... s4:ldb - SQLite: port some constraints from the TDB backend also to the SQLITE one
       via  75eff6e... s4:subtree_delete - Make the initialisation of the child counter more clear
       via  30faff7... s4:ldap.py - Further enhancements
       via  f9990e9... s4:ldb - add a check which has to be done on beginning of a "modify" operation
       via  ee0204c... s4:ldap server - remove unused error handlings
       via  32a7b82... s4:ldb_tdb - Rework/Various
      from  746fb5a... Correct fix for bug 6781 - Cannot rename subfolders in Explorer view with recent versions of Samba. Without this fix, renaming a directory ./a to ./b, whilst a directory ./aa was already open would fail. Jeremy.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 91456b8dc5c4237eb76ac82afa0c5f4a35bdca65
Author: Matthias Dieter Wallnöfer <mwallnoefer at yahoo.de>
Date:   Tue Oct 6 09:28:38 2009 +0200

    s4:ldb - SQLite: port some constraints from the TDB backend also to the SQLITE one

commit 75eff6eaf3df69a397980f0717fc5cc720cfe0b9
Author: Matthias Dieter Wallnöfer <mwallnoefer at yahoo.de>
Date:   Wed Oct 7 12:38:00 2009 +0200

    s4:subtree_delete - Make the initialisation of the child counter more clear

commit 30faff7567f2a7e82a6496bbf221cd8de5d5b50d
Author: Matthias Dieter Wallnöfer <mwallnoefer at yahoo.de>
Date:   Tue Oct 6 17:18:04 2009 +0200

    s4:ldap.py - Further enhancements
    
    - Enhance test for "distinguishedName"
    - Add a test for single-valued attributes
    - Add a test for multi-valued attributes
    - Add a test for empty messages
    - Add a test for empty attributes

commit f9990e9b391f330a8e6c5c158ee4e4eaa50f6176
Author: Matthias Dieter Wallnöfer <mwallnoefer at yahoo.de>
Date:   Wed Oct 7 23:49:29 2009 +0200

    s4:ldb - add a check which has to be done on beginning of a "modify" operation

commit ee0204cfccbd73050b2ec806f392bf5c4a549430
Author: Matthias Dieter Wallnöfer <mwallnoefer at yahoo.de>
Date:   Tue Oct 6 21:53:05 2009 +0200

    s4:ldap server - remove unused error handlings
    
    Those error cases should be handled by LDB itself to be available on all
    connection methods and not only over LDAP.

commit 32a7b82e87c71e103c47fee787ed81db6266921f
Author: Matthias Dieter Wallnöfer <mwallnoefer at yahoo.de>
Date:   Tue Oct 6 09:30:53 2009 +0200

    s4:ldb_tdb - Rework/Various
    
    - Unify the error handling method with "done" mark in all longer functions
    - Fix up result codes to match more the real MS AD
    - Some cosmetic fixups

-----------------------------------------------------------------------

Summary of changes:
 source4/dsdb/samdb/ldb_modules/subtree_delete.c |    2 +
 source4/ldap_server/ldap_backend.c              |   35 --
 source4/lib/ldb/common/ldb.c                    |    8 +
 source4/lib/ldb/ldb_sqlite3/ldb_sqlite3.c       |   49 +++
 source4/lib/ldb/ldb_tdb/ldb_tdb.c               |  392 ++++++++++++-----------
 source4/lib/ldb/tests/python/ldap.py            |  242 ++++++++++++++-
 6 files changed, 495 insertions(+), 233 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source4/dsdb/samdb/ldb_modules/subtree_delete.c b/source4/dsdb/samdb/ldb_modules/subtree_delete.c
index 241cc5f..e1ce9c1 100644
--- a/source4/dsdb/samdb/ldb_modules/subtree_delete.c
+++ b/source4/dsdb/samdb/ldb_modules/subtree_delete.c
@@ -55,6 +55,8 @@ static struct subtree_delete_context *subdel_ctx_init(struct ldb_module *module,
 	ac->module = module;
 	ac->req = req;
 
+	ac->num_children = 0;
+
 	return ac;
 }
 
diff --git a/source4/ldap_server/ldap_backend.c b/source4/ldap_server/ldap_backend.c
index 5f9b822..f3d82a7 100644
--- a/source4/ldap_server/ldap_backend.c
+++ b/source4/ldap_server/ldap_backend.c
@@ -534,26 +534,11 @@ static NTSTATUS ldapsrv_ModifyRequest(struct ldapsrv_call *call)
 				NT_STATUS_HAVE_NO_MEMORY(msg->elements[i].values);
 
 				for (j=0; j < msg->elements[i].num_values; j++) {
-					if (!(req->mods[i].attrib.values[j].length > 0)) {
-						result = LDAP_OTHER;
-
-						map_ldb_error(local_ctx,
-							LDB_ERR_OTHER, &errstr);
-						errstr = talloc_asprintf(local_ctx,
-							"%s. Empty attribute values not allowed", errstr);
-						goto reply;
-					}
 					msg->elements[i].values[j].length = req->mods[i].attrib.values[j].length;
 					msg->elements[i].values[j].data = req->mods[i].attrib.values[j].data;			
 				}
 			}
 		}
-	} else {
-		result = LDAP_OTHER;
-		map_ldb_error(local_ctx, LDB_ERR_OTHER, &errstr);
-		errstr = talloc_asprintf(local_ctx,
-			"%s. No mods are not allowed", errstr);
-		goto reply;
 	}
 
 reply:
@@ -628,31 +613,11 @@ static NTSTATUS ldapsrv_AddRequest(struct ldapsrv_call *call)
 				NT_STATUS_HAVE_NO_MEMORY(msg->elements[i].values);
 
 				for (j=0; j < msg->elements[i].num_values; j++) {
-					if (!(req->attributes[i].values[j].length > 0)) {
-						result = LDAP_OTHER;
-						map_ldb_error(local_ctx,
-							LDB_ERR_OTHER, &errstr);
-						errstr = talloc_asprintf(local_ctx,
-							"%s. Empty attribute values not allowed", errstr);
-						goto reply;
-					}
 					msg->elements[i].values[j].length = req->attributes[i].values[j].length;
 					msg->elements[i].values[j].data = req->attributes[i].values[j].data;			
 				}
-			} else {
-				result = LDAP_OTHER;
-				map_ldb_error(local_ctx, LDB_ERR_OTHER, &errstr);
-				errstr = talloc_asprintf(local_ctx,
-					"%s. No attribute values are not allowed", errstr);
-				goto reply;
 			}
 		}
-	} else {
-		result = LDAP_OTHER;
-		map_ldb_error(local_ctx, LDB_ERR_OTHER, &errstr);
-		errstr = talloc_asprintf(local_ctx, 
-			"%s. No attributes are not allowed", errstr);
-		goto reply;
 	}
 
 reply:
diff --git a/source4/lib/ldb/common/ldb.c b/source4/lib/ldb/common/ldb.c
index e9c9245..4c27de7 100644
--- a/source4/lib/ldb/common/ldb.c
+++ b/source4/lib/ldb/common/ldb.c
@@ -1358,6 +1358,14 @@ int ldb_modify(struct ldb_context *ldb,
 		return ret;
 	}
 
+	if (message->num_elements == 0) {
+		/* this needs also to be returned when the specified object
+		   doesn't exist. Therefore this test is located here. */
+		ldb_asprintf_errstring(ldb, "LDB message has to have elements/attributes (%s)!",
+				       ldb_dn_get_linearized(message->dn));
+		return LDB_ERR_UNWILLING_TO_PERFORM;
+	}
+
 	ret = ldb_build_mod_req(&req, ldb, ldb,
 					message,
 					NULL,
diff --git a/source4/lib/ldb/ldb_sqlite3/ldb_sqlite3.c b/source4/lib/ldb/ldb_sqlite3/ldb_sqlite3.c
index d0573d3..7e420e4 100644
--- a/source4/lib/ldb/ldb_sqlite3/ldb_sqlite3.c
+++ b/source4/lib/ldb/ldb_sqlite3/ldb_sqlite3.c
@@ -1053,6 +1053,19 @@ static int lsql_add(struct lsql_context *ctx)
 
 		a = ldb_schema_attribute_by_name(ldb, el->name);
 
+		if (el->num_value == 0) {
+			ldb_asprintf_errstring(ldb, "attribute %s on %s specified, but with 0 values (illegal)",
+					       el->name, ldb_dn_get_linearized(msg->dn));
+			return LDB_ERR_CONSTRAINT_VIOLATION;
+		}
+		if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) {
+			if (el->num_values > 1) {
+				ldb_asprintf_errstring(ldb, "SINGLE-VALUED attribute %s on %s specified more than once",
+						       el->name, ldb_dn_get_linearized(msg->dn));
+				return LDB_ERR_CONSTRAINT_VIOLATION;
+			}
+		}
+
 		/* For each value of the specified attribute name... */
 		for (j = 0; j < el->num_values; j++) {
 			struct ldb_val value;
@@ -1125,6 +1138,12 @@ static int lsql_modify(struct lsql_context *ctx)
 		char *mod;
 		int j;
 
+		if (ldb_attr_cmp(el->name, "distinguishedName") == 0) {
+			ldb_asprintf_errstring(ldb, "it is not permitted to perform a modify on 'distinguishedName' (use rename instead): %s",
+					       ldb_dn_get_linearized(msg->dn));
+			return LDB_ERR_CONSTRAINT_VIOLATION;
+		}
+
 		/* Get a case-folded copy of the attribute name */
 		attr = ldb_attr_casefold(ctx, el->name);
 		if (attr == NULL) {
@@ -1137,6 +1156,21 @@ static int lsql_modify(struct lsql_context *ctx)
 
 		case LDB_FLAG_MOD_REPLACE:
 
+			if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) {
+				if (el->num_values > 1) {
+					ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s specified more than once",
+						               el->name, ldb_dn_get_linearized(msg->dn));
+					return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS;
+				}
+			}
+
+			for (j=0; j<el->num_values; j++) {
+				if (ldb_msg_find_val(el, &el->values[j]) != &el->values[j]) {
+					ldb_asprintf_errstring(ldb, "%s: value #%d provided more than once", el->name, j);
+					return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS;
+				}
+			}
+
 			/* remove all attributes before adding the replacements */
 			mod = lsqlite3_tprintf(ctx,
 						"DELETE FROM ldb_attribute_values "
@@ -1159,6 +1193,21 @@ static int lsql_modify(struct lsql_context *ctx)
 			/* MISSING break is INTENTIONAL */
 
 		case LDB_FLAG_MOD_ADD:
+
+			if (el->num_values == 0) {
+				ldb_asprintf_errstring(ldb, "attribute %s on %s specified, but with 0 values (illigal)",
+						       el->name, ldb_dn_get_linearized(msg->dn));
+				return LDB_ERR_CONSTRAINT_VIOLATION;
+			}
+
+			if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) {
+				if (el->num_values > 1) {
+					ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s specified more than once",
+						               el->name, ldb_dn_get_linearized(msg->dn));
+					return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS;
+				}
+			}
+
 #warning "We should throw an error if no value is provided!"
 			/* For each value of the specified attribute name... */
 			for (j = 0; j < el->num_values; j++) {
diff --git a/source4/lib/ldb/ldb_tdb/ldb_tdb.c b/source4/lib/ldb/ldb_tdb/ldb_tdb.c
index 66a10b6..7693ffe 100644
--- a/source4/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/source4/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1,10 +1,10 @@
 /*
    ldb database library
 
-   Copyright (C) Andrew Tridgell  2004
-   Copyright (C) Stefan Metzmacher  2004
-   Copyright (C) Simo Sorce       2006-2008
-
+   Copyright (C) Andrew Tridgell 2004
+   Copyright (C) Stefan Metzmacher 2004
+   Copyright (C) Simo Sorce 2006-2008
+   Copyright (C) Matthias Dieter Wallnöfer 2009
 
      ** NOTE! The following LGPL license applies to the ldb
      ** library. This does NOT imply that all of Samba is released
@@ -43,6 +43,10 @@
  *  - description: make it possible to use event contexts
  *    date: Jan 2008
  *    Author: Simo Sorce
+ *
+ *  - description: fix up memory leaks and small bugs
+ *    date: Oct 2009
+ *    Author: Matthias Dieter Wallnöfer
  */
 
 #include "ldb_tdb.h"
@@ -169,7 +173,7 @@ static int ltdb_check_special_dn(struct ldb_module *module,
 
 	if (! ldb_dn_is_special(msg->dn) ||
 	    ! ldb_dn_check_special(msg->dn, LTDB_ATTRIBUTES)) {
-		return 0;
+		return LDB_SUCCESS;
 	}
 
 	/* we have @ATTRIBUTES, let's check attributes are fine */
@@ -183,7 +187,7 @@ static int ltdb_check_special_dn(struct ldb_module *module,
 		}
 	}
 
-	return 0;
+	return LDB_SUCCESS;
 }
 
 
@@ -218,7 +222,7 @@ int ltdb_store(struct ldb_module *module, const struct ldb_message *msg, int flg
 	void *data = ldb_module_get_private(module);
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
 	TDB_DATA tdb_key, tdb_data;
-	int ret;
+	int ret = LDB_SUCCESS;
 
 	tdb_key = ltdb_key(module, msg->dn);
 	if (!tdb_key.dptr) {
@@ -254,15 +258,16 @@ static int ltdb_add_internal(struct ldb_module *module,
 			     const struct ldb_message *msg)
 {
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
-	int ret, i;
+	int ret = LDB_SUCCESS, i;
 
 	ret = ltdb_check_special_dn(module, msg);
 	if (ret != LDB_SUCCESS) {
-		return ret;
+		goto done;
 	}
 
 	if (ltdb_cache_load(module) != 0) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		goto done;
 	}
 
 	for (i=0;i<msg->num_elements;i++) {
@@ -272,38 +277,40 @@ static int ltdb_add_internal(struct ldb_module *module,
 		if (el->num_values == 0) {
 			ldb_asprintf_errstring(ldb, "attribute %s on %s specified, but with 0 values (illegal)", 
 					       el->name, ldb_dn_get_linearized(msg->dn));
-			return LDB_ERR_CONSTRAINT_VIOLATION;
+			ret = LDB_ERR_CONSTRAINT_VIOLATION;
+			goto done;
 		}
 		if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) {
 			if (el->num_values > 1) {
-				ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s speicified more than once", 
+				ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s specified more than once",
 						       el->name, ldb_dn_get_linearized(msg->dn));
-				return LDB_ERR_CONSTRAINT_VIOLATION;
+				ret = LDB_ERR_CONSTRAINT_VIOLATION;
+				goto done;
 			}
 		}
 	}
 
 	ret = ltdb_store(module, msg, TDB_INSERT);
-
-	if (ret == LDB_ERR_ENTRY_ALREADY_EXISTS) {
-		ldb_asprintf_errstring(ldb,
-					"Entry %s already exists",
-					ldb_dn_get_linearized(msg->dn));
-		return ret;
+	if (ret != LDB_SUCCESS) {
+		if (ret == LDB_ERR_ENTRY_ALREADY_EXISTS) {
+			ldb_asprintf_errstring(ldb,
+					       "Entry %s already exists",
+					       ldb_dn_get_linearized(msg->dn));
+		}
+		goto done;
 	}
 
-	if (ret == LDB_SUCCESS) {
-		ret = ltdb_index_one(module, msg, 1);
-		if (ret != LDB_SUCCESS) {
-			return ret;
-		}
+	ret = ltdb_index_one(module, msg, 1);
+	if (ret != LDB_SUCCESS) {
+		goto done;
+	}
 
-		ret = ltdb_modified(module, msg->dn);
-		if (ret != LDB_SUCCESS) {
-			return ret;
-		}
+	ret = ltdb_modified(module, msg->dn);
+	if (ret != LDB_SUCCESS) {
+		goto done;
 	}
 
+done:
 	return ret;
 }
 
@@ -314,16 +321,17 @@ static int ltdb_add(struct ltdb_context *ctx)
 {
 	struct ldb_module *module = ctx->module;
 	struct ldb_request *req = ctx->req;
-	int tret;
+	int ret = LDB_SUCCESS;
 
 	ldb_request_set_state(req, LDB_ASYNC_PENDING);
 
-	tret = ltdb_add_internal(module, req->op.add.message);
-	if (tret != LDB_SUCCESS) {
-		return tret;
+	if (ltdb_cache_load(module) != 0) {
+		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	return LDB_SUCCESS;
+	ret = ltdb_add_internal(module, req->op.add.message);
+
+	return ret;
 }
 
 /*
@@ -355,7 +363,7 @@ int ltdb_delete_noindex(struct ldb_module *module, struct ldb_dn *dn)
 static int ltdb_delete_internal(struct ldb_module *module, struct ldb_dn *dn)
 {
 	struct ldb_message *msg;
-	int ret;
+	int ret = LDB_SUCCESS;
 
 	msg = talloc(module, struct ldb_message);
 	if (msg == NULL) {
@@ -404,7 +412,7 @@ static int ltdb_delete(struct ltdb_context *ctx)
 {
 	struct ldb_module *module = ctx->module;
 	struct ldb_request *req = ctx->req;
-	int tret;
+	int ret = LDB_SUCCESS;
 
 	ldb_request_set_state(req, LDB_ASYNC_PENDING);
 
@@ -412,12 +420,9 @@ static int ltdb_delete(struct ltdb_context *ctx)
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	tret = ltdb_delete_internal(module, req->op.del.dn);
-	if (tret != LDB_SUCCESS) {
-		return tret;
-	}
+	ret = ltdb_delete_internal(module, req->op.del.dn);
 
-	return LDB_SUCCESS;
+	return ret;
 }
 
 /*
@@ -453,6 +458,11 @@ static int msg_add_element(struct ldb_context *ldb,
 	struct ldb_message_element *e2;
 	unsigned int i;
 
+	if (el->num_values == 0) {
+		/* nothing to do here - we don't add empty elements */
+		return 0;
+	}
+
 	e2 = talloc_realloc(msg, msg->elements, struct ldb_message_element,
 			      msg->num_elements+1);
 	if (!e2) {
@@ -466,21 +476,18 @@ static int msg_add_element(struct ldb_context *ldb,
 
 	e2->name = el->name;
 	e2->flags = el->flags;
-	e2->values = NULL;
-	if (el->num_values != 0) {
-		e2->values = talloc_array(msg->elements,
-					  struct ldb_val, el->num_values);
-		if (!e2->values) {
-			errno = ENOMEM;
-			return -1;
-		}
+	e2->values = talloc_array(msg->elements,
+				  struct ldb_val, el->num_values);
+	if (!e2->values) {
+		errno = ENOMEM;
+		return -1;
 	}
 	for (i=0;i<el->num_values;i++) {
 		e2->values[i] = el->values[i];
 	}
 	e2->num_values = el->num_values;
 
-	msg->num_elements++;
+	++msg->num_elements;
 
 	return 0;
 }
@@ -516,12 +523,17 @@ static int msg_delete_attribute(struct ldb_module *module,
 			msg->num_elements--;
 			i--;
 			msg->elements = talloc_realloc(msg, msg->elements,
-							struct ldb_message_element,
-							msg->num_elements);
+						       struct ldb_message_element,
+						       msg->num_elements);
+
+			/* per definition we find in a canonicalised message an
+			   attribute only once. So we are finished here. */
+			return 0;
 		}
 	}
 
-	return 0;
+	/* Not found */
+	return -1;
 }
 
 /*
@@ -562,10 +574,14 @@ static int msg_delete_element(struct ldb_module *module,
 				return msg_delete_attribute(module, ldb,
 							    msg, name);
 			}
+
+			/* per definition we find in a canonicalised message an
+			   attribute value only once. So we are finished here */
 			return 0;
 		}
 	}
 
+	/* Not found */
 	return -1;
 }
 
@@ -586,7 +602,7 @@ int ltdb_modify_internal(struct ldb_module *module,
 	TDB_DATA tdb_key, tdb_data;
 	struct ldb_message *msg2;
 	unsigned i, j;
-	int ret, idx;
+	int ret = LDB_SUCCESS, idx;
 
 	tdb_key = ltdb_key(module, msg->dn);
 	if (!tdb_key.dptr) {
@@ -602,172 +618,174 @@ int ltdb_modify_internal(struct ldb_module *module,
 	msg2 = talloc(tdb_key.dptr, struct ldb_message);
 	if (msg2 == NULL) {
 		free(tdb_data.dptr);
-		talloc_free(tdb_key.dptr);
-		return LDB_ERR_OTHER;
+		ret = LDB_ERR_OTHER;
+		goto done;
 	}
 
 	ret = ltdb_unpack_data(module, &tdb_data, msg2);
 	free(tdb_data.dptr);
 	if (ret == -1) {
 		ret = LDB_ERR_OTHER;
-		goto failed;
+		goto done;
 	}
 
 	if (!msg2->dn) {
 		msg2->dn = msg->dn;
 	}
 
-	for (i=0;i<msg->num_elements;i++) {
-		struct ldb_message_element *el = &msg->elements[i];
-		struct ldb_message_element *el2;
+	for (i=0; i<msg->num_elements; i++) {
+		struct ldb_message_element *el = &msg->elements[i], *el2;
 		struct ldb_val *vals;
-		const char *dn;
 		const struct ldb_schema_attribute *a = ldb_schema_attribute_by_name(ldb, el->name);
+		const char *dn;
 
 		if (ldb_attr_cmp(el->name, "distinguishedName") == 0) {
-			ldb_asprintf_errstring(ldb, "it is not permitted to perform a modify on distinguishedName (use rename instead): %s",
+			ldb_asprintf_errstring(ldb, "it is not permitted to perform a modify on 'distinguishedName' (use rename instead): %s",
 					       ldb_dn_get_linearized(msg->dn));
-			ret = LDB_ERR_UNWILLING_TO_PERFORM;
-			goto failed;
+			ret = LDB_ERR_CONSTRAINT_VIOLATION;
+			goto done;
 		}
 
 		switch (msg->elements[i].flags & LDB_FLAG_MOD_MASK) {
 		case LDB_FLAG_MOD_ADD:
-			
-			/* add this element to the message. fail if it
-			   already exists */


-- 
Samba Shared Repository


More information about the samba-cvs mailing list