From 1378b056d9662a5667e86f3834e0d82c1610e6a6 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Fri, 29 Jan 2010 17:27:52 -0800 Subject: 559016 - Attempting to rename suffix returns inappropriate errors https://bugzilla.redhat.com/show_bug.cgi?id=559016 [Fix Description] If the target dn of the modrdn operation is a suffix, check if the new dn already exists or not. If it exists, it returns LDAP_ALREADY_EXISTS. If the backend associated with the new dn does not exist, it returns LDAP_NO_SUCH_OBJECT. Otherwise, it returns LDAP_NAMING_VIOLATION. If the target dn of the modrdn is attempted to move across backends, it returns LDAP_AFFECTS_MULTIPLE_DSAS instead of LDAP_UNWILLING_TO_PERFORM. Modrdn (op_shared_rename) was logging the parameter errors in the clients request as SLAPI_LOG_FATAL. Reduced the level to SLAPI_LOG_ARGS. Also, replaced ldap_explode_dn with slapi_dn_syntax_check to verify the newsuperior. By the replacement, 2 bugs in slapi_dn_syntax_check were found. 1) The key for the DN in the hashtable of the attribute syntax has to be "distinguishedName". 2) Argument type for plg_syntax_validate was not correct. --- ldap/servers/slapd/mapping_tree.c | 51 +++++++++++++++++++++++++++++++++++--- ldap/servers/slapd/modrdn.c | 25 ++++++++----------- ldap/servers/slapd/plugin_syntax.c | 4 +-- 3 files changed, 60 insertions(+), 20 deletions(-) diff --git a/ldap/servers/slapd/mapping_tree.c b/ldap/servers/slapd/mapping_tree.c index 3b082074..8b4c541b 100644 --- a/ldap/servers/slapd/mapping_tree.c +++ b/ldap/servers/slapd/mapping_tree.c @@ -2262,11 +2262,54 @@ int slapi_mapping_tree_select_and_check(Slapi_PBlock *pb,char *newdn, Slapi_Back if (ret) goto unlock_and_return; - if ((*be) && ((*be != new_be) || mtn_sdn_has_child(target_sdn))) + if (*be) { - ret = LDAP_UNWILLING_TO_PERFORM; - PR_snprintf(errorbuf, BUFSIZ, "Cannot move entries accross backends\n"); - goto unlock_and_return; + /* suffix is a part of mapping tree. We should not free it */ + const Slapi_DN *suffix = slapi_get_suffix_by_dn(target_sdn); + if (NULL == suffix) + { + ret = LDAP_NO_SUCH_OBJECT; + PR_snprintf(errorbuf, BUFSIZ, + "Target entry \"%s\" does not exist\n", + slapi_sdn_get_dn(target_sdn)); + goto unlock_and_return; + } + if (0 == slapi_sdn_compare(target_sdn, suffix)) + { + /* target_sdn is a suffix */ + const Slapi_DN *new_suffix = NULL; + /* new_suffix is a part of mapping tree. We should not free it */ + new_suffix = slapi_get_suffix_by_dn(&dn_newdn); + if (!slapi_be_exist((const Slapi_DN *)&dn_newdn)) + { + /* new_be is an empty backend */ + ret = LDAP_NO_SUCH_OBJECT; + PR_snprintf(errorbuf, BUFSIZ, + "Backend for suffix \"%s\" does not exist\n", newdn); + goto unlock_and_return; + } + if (0 == slapi_sdn_compare(&dn_newdn, new_suffix)) + { + ret = LDAP_ALREADY_EXISTS; + PR_snprintf(errorbuf, BUFSIZ, + "Suffix \"%s\" already exists\n", newdn); + goto unlock_and_return; + } + ret = LDAP_NAMING_VIOLATION; + PR_snprintf(errorbuf, BUFSIZ, "Cannot rename suffix \"%s\"\n", + slapi_sdn_get_dn(target_sdn)); + goto unlock_and_return; + } + else + { + if ((*be != new_be) || mtn_sdn_has_child(target_sdn)) + { + ret = LDAP_AFFECTS_MULTIPLE_DSAS; + PR_snprintf(errorbuf, BUFSIZ, + "Cannot move entries accross backends\n"); + goto unlock_and_return; + } + } } unlock_and_return: diff --git a/ldap/servers/slapd/modrdn.c b/ldap/servers/slapd/modrdn.c index 8f332a10..38d13000 100644 --- a/ldap/servers/slapd/modrdn.c +++ b/ldap/servers/slapd/modrdn.c @@ -352,7 +352,7 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args) { if ( !internal_op ) { - slapi_log_access(LDAP_DEBUG_STATS, + slapi_log_access(SLAPI_LOG_ARGS, "conn=%" NSPRIu64 " op=%d MODRDN dn=\"%s\" newrdn=\"%s\" newsuperior=\"%s\"\n", pb->pb_conn->c_connid, pb->pb_op->o_opid, @@ -362,7 +362,7 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args) } else { - slapi_log_access(LDAP_DEBUG_ARGS, + slapi_log_access(SLAPI_LOG_ARGS, "conn=%s op=%d MODRDN dn=\"%s\" newrdn=\"%s\" newsuperior=\"%s\"\n", LOG_INTERNAL_OP_CON_ID, LOG_INTERNAL_OP_OP_ID, @@ -376,13 +376,13 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args) if ((rdns = ldap_explode_rdn(newrdn, 0)) == NULL) { if ( !internal_op ) { - slapi_log_error(SLAPI_LOG_FATAL, NULL, + slapi_log_error(SLAPI_LOG_ARGS, NULL, "conn=%" NSPRIu64 " op=%d MODRDN invalid new RDN (\"%s\")\n", pb->pb_conn->c_connid, pb->pb_op->o_opid, (NULL == newrdn) ? "(null)" : newrdn); } else { - slapi_log_error(SLAPI_LOG_FATAL, NULL, + slapi_log_error(SLAPI_LOG_ARGS, NULL, "conn=%" NSPRIu64 " op=%d MODRDN invalid new RDN (\"%s\")\n", LOG_INTERNAL_OP_CON_ID, LOG_INTERNAL_OP_OP_ID, @@ -403,30 +403,27 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args) } /* check that the dn is formatted correctly */ - if ((rdns = ldap_explode_dn(newsuperior, 0)) == NULL) + err = slapi_dn_syntax_check(pb, newsuperior, 1); + if (err) { - LDAPDebug(LDAP_DEBUG_ARGS, "ldap_explode_dn of newSuperior failed\n", 0, 0, 0); + LDAPDebug0Args(LDAP_DEBUG_ARGS, "Syntax check of newSuperior failed\n"); if (!internal_op) { - slapi_log_error(SLAPI_LOG_FATAL, NULL, + slapi_log_error(SLAPI_LOG_ARGS, NULL, "conn=%" NSPRIu64 " op=%d MODRDN invalid new superior (\"%s\")", pb->pb_conn->c_connid, pb->pb_op->o_opid, (NULL == newsuperior) ? "(null)" : newsuperiorbuf); } else { - slapi_log_error(SLAPI_LOG_FATAL, NULL, + slapi_log_error(SLAPI_LOG_ARGS, NULL, "conn=%" NSPRIu64 " op=%d MODRDN invalid new superior (\"%s\")", LOG_INTERNAL_OP_CON_ID, LOG_INTERNAL_OP_OP_ID, (NULL == newsuperior) ? "(null)" : newsuperiorbuf); } - send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL, + send_ldap_result(pb, LDAP_INVALID_DN_SYNTAX, NULL, "newSuperior does not look like a DN", 0, NULL); goto free_and_return_nolock; } - else - { - slapi_ldap_value_free(rdns); - } if (newsuperior != NULL) { @@ -448,7 +445,7 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args) * if we don't hold it. */ if ((err = slapi_mapping_tree_select_and_check(pb, newdn, &be, &referral, errorbuf)) != LDAP_SUCCESS) - { + { send_ldap_result(pb, err, NULL, errorbuf, 0, NULL); goto free_and_return_nolock; } diff --git a/ldap/servers/slapd/plugin_syntax.c b/ldap/servers/slapd/plugin_syntax.c index ecf999bc..e2cc7fb1 100644 --- a/ldap/servers/slapd/plugin_syntax.c +++ b/ldap/servers/slapd/plugin_syntax.c @@ -295,7 +295,7 @@ slapi_dn_syntax_check( } /* Locate the dn syntax plugin. */ - slapi_attr_type2plugin("dn", (void **)&dn_plugin); + slapi_attr_type2plugin("distinguishedName", (void **)&dn_plugin); /* Assume the value is valid if we don't find a dn validate function */ if (dn_plugin && dn_plugin->plg_syntax_validate != NULL) { @@ -305,7 +305,7 @@ slapi_dn_syntax_check( dn_bval.bv_len = strlen(dn); /* Validate the value. */ - if (dn_plugin->plg_syntax_validate(dn_bval) != 0) { + if (dn_plugin->plg_syntax_validate(&dn_bval) != 0) { if (syntaxlogging) { slapi_log_error( SLAPI_LOG_FATAL, "Syntax Check", "DN value (%s) invalid per syntax\n", dn ? dn : ""); -- cgit