Revert "s4:samldb LDB module - simplify the message handling on add and modify operations"

This reverts commit 1d94bb3ad4d9c6de3b77ed4690a54ebf2399cc0d.

This commit causes unconditional behaviour (sometimes it works, sometimes not) -sorry for introducing this.

I will rework this further.

1 files changed, 33 insertions, 26 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 38ac5fbde6..7562122016 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -1077,9 +1077,11 @@ static int samldb_add(struct ldb_module *module, struct ldb_request *req)
 	}
 
 	/* build the new msg */
-	req->op.add.message = ac->msg = ldb_msg_copy_shallow(req,
-							     req->op.add.message);
-	if (ac->msg == NULL) {
+	ac->msg = ldb_msg_copy(ac, ac->req->op.add.message);
+	if (!ac->msg) {
+		talloc_free(ac);
+		ldb_debug(ldb, LDB_DEBUG_FATAL,
+			  "samldb_add: ldb_msg_copy failed!\n");
 		return ldb_operr(ldb);
 	}
 
@@ -1133,6 +1135,7 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
 {
 	struct ldb_context *ldb;
 	struct samldb_ctx *ac;
+	struct ldb_message *msg;
 	struct ldb_message_element *el, *el2;
 	int ret;
 	uint32_t account_type;
@@ -1163,34 +1166,35 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
 		return ldb_operr(ldb);
 	}
 
-	/* build the new msg */
-	req->op.mod.message = ac->msg = ldb_msg_copy_shallow(req,
-							     req->op.mod.message);
-	if (ac->msg == NULL) {
-		return ldb_operr(ldb);
-	}
+	/* TODO: do not modify original request, create a new one */
 
-	el = ldb_msg_find_element(ac->msg, "groupType");
+	el = ldb_msg_find_element(req->op.mod.message, "groupType");
 	if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_REPLACE) && el->num_values == 1) {
 		uint32_t group_type;
 
+		req->op.mod.message = msg = ldb_msg_copy_shallow(req,
+			req->op.mod.message);
+
 		group_type = strtoul((const char *)el->values[0].data, NULL, 0);
 		account_type =  ds_gtype2atype(group_type);
-		ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
+		ret = samdb_msg_add_uint(ldb, msg, msg,
 					 "sAMAccountType",
 					 account_type);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
-		el2 = ldb_msg_find_element(ac->msg, "sAMAccountType");
+		el2 = ldb_msg_find_element(msg, "sAMAccountType");
 		el2->flags = LDB_FLAG_MOD_REPLACE;
 	}
 	if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE)) {
 		return LDB_ERR_UNWILLING_TO_PERFORM;
 	}
 
-	el = ldb_msg_find_element(ac->msg, "primaryGroupID");
+	el = ldb_msg_find_element(req->op.mod.message, "primaryGroupID");
 	if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_REPLACE) && el->num_values == 1) {
+		req->op.mod.message = ac->msg = ldb_msg_copy_shallow(req,
+			req->op.mod.message);
+
 		ret = samldb_prim_group_change(ac);
 		if (ret != LDB_SUCCESS) {
 			return ret;
@@ -1200,35 +1204,36 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
 		return LDB_ERR_UNWILLING_TO_PERFORM;
 	}
 
-	el = ldb_msg_find_element(ac->msg, "userAccountControl");
+	el = ldb_msg_find_element(req->op.mod.message, "userAccountControl");
 	if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_REPLACE) && el->num_values == 1) {
 		uint32_t user_account_control;
 
+		req->op.mod.message = msg = ldb_msg_copy_shallow(req,
+			req->op.mod.message);
+
 		user_account_control = strtoul((const char *)el->values[0].data,
 			NULL, 0);
 		account_type = ds_uf2atype(user_account_control);
-		ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
+		ret = samdb_msg_add_uint(ldb, msg, msg,
 					 "sAMAccountType",
 					 account_type);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
-		el2 = ldb_msg_find_element(ac->msg, "sAMAccountType");
+		el2 = ldb_msg_find_element(msg, "sAMAccountType");
 		el2->flags = LDB_FLAG_MOD_REPLACE;
 
 		if (user_account_control & (UF_SERVER_TRUST_ACCOUNT | UF_PARTIAL_SECRETS_ACCOUNT)) {
-			ret = samdb_msg_add_string(ldb, ac->msg, ac->msg,
-						   "isCriticalSystemObject",
-						   "TRUE");
+			ret = samdb_msg_add_string(ldb, msg, msg,
+						   "isCriticalSystemObject", "TRUE");
 			if (ret != LDB_SUCCESS) {
 				return ret;
 			}
-			el2 = ldb_msg_find_element(ac->msg,
-						   "isCriticalSystemObject");
+			el2 = ldb_msg_find_element(msg, "isCriticalSystemObject");
 			el2->flags = LDB_FLAG_MOD_REPLACE;
 
 			/* DCs have primaryGroupID of DOMAIN_RID_DCS */
-			if (!ldb_msg_find_element(ac->msg, "primaryGroupID")) {
+			if (!ldb_msg_find_element(msg, "primaryGroupID")) {
 				uint32_t rid;
 				if (user_account_control & UF_SERVER_TRUST_ACCOUNT) {
 					rid = DOMAIN_RID_DCS;
@@ -1236,13 +1241,12 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
 					/* read-only DC */
 					rid = DOMAIN_RID_READONLY_DCS;
 				}
-				ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
+				ret = samdb_msg_add_uint(ldb, msg, msg,
 							 "primaryGroupID", rid);
 				if (ret != LDB_SUCCESS) {
 					return ret;
 				}
-				el2 = ldb_msg_find_element(ac->msg,
-							   "primaryGroupID");
+				el2 = ldb_msg_find_element(msg, "primaryGroupID");
 				el2->flags = LDB_FLAG_MOD_REPLACE;
 			}
 		}
@@ -1251,8 +1255,11 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
 		return LDB_ERR_UNWILLING_TO_PERFORM;
 	}
 
-	el = ldb_msg_find_element(ac->msg, "member");
+	el = ldb_msg_find_element(req->op.mod.message, "member");
 	if (el && el->flags & (LDB_FLAG_MOD_ADD|LDB_FLAG_MOD_REPLACE) && el->num_values == 1) {
+		req->op.mod.message = ac->msg = ldb_msg_copy_shallow(req,
+			req->op.mod.message);
+
 		ret = samldb_member_check(ac);
 		if (ret != LDB_SUCCESS) {
 			return ret;