[389-commits] ldap/servers

[Mark Reynolds]

 ldap/servers/plugins/replication/repl5_agmtlist.c       |   32 ++++++++--------
 ldap/servers/plugins/replication/repl5_replica_config.c |   12 ++++--
 2 files changed, 25 insertions(+), 19 deletions(-)

New commits:
commit 8a4bbc7c74a6847d75e4d6e9e0b16859a5da8ec0
Author: Mark Reynolds <mreynolds at redhat.com>
Date:   Fri Dec 13 11:43:47 2013 -0500

    Ticket 47620 - Config value validation improvement
    
    Bug Description:  When setting the replication protocol timeout, it is possible
                      to set a negative number(it should be rejected), and when
                      setting the timeout for an agreement using letters, we get an
                      invalid syntax error, but it should really be an error 53 to
                      be consistent with how the invalid timeout error that is given
                      when updating the replica entry.
    
    Fix Description:  In the agmt modify code, we did not have the actual modify value
                      during the validation.  This allowed the value to be added, which
                      was later caught for the invalid syntax.  Then improved the overall
                      logic to the validation to also catch the negative numbers.
    
    https://fedorahosted.org/389/ticket/47620
    
    Reviewed by: rmeggins(Thanks!)

diff --git a/ldap/servers/plugins/replication/repl5_agmtlist.c b/ldap/servers/plugins/replication/repl5_agmtlist.c
index f9b844b..7d1c651 100644
--- a/ldap/servers/plugins/replication/repl5_agmtlist.c
+++ b/ldap/servers/plugins/replication/repl5_agmtlist.c
@@ -245,6 +245,7 @@ agmtlist_modify_callback(Slapi_PBlock *pb, Slapi_Entry *entryBefore, Slapi_Entry
 	for (i = 0; NULL != mods && NULL != mods[i]; i++)
 	{
 		slapi_ch_free_string(&val);
+		val = slapi_berval_get_string_copy (mods[i]->mod_bvalues[0]);
 		if (slapi_attr_types_equivalent(mods[i]->mod_type, type_nsds5ReplicaInitialize))
 		{
             /* we don't allow delete attribute operations unless it was issued by
@@ -268,10 +269,7 @@ agmtlist_modify_callback(Slapi_PBlock *pb, Slapi_Entry *entryBefore, Slapi_Entry
             }
             else
             {
-                if (mods[i]->mod_bvalues && mods[i]->mod_bvalues[0])
-                    val = slapi_berval_get_string_copy (mods[i]->mod_bvalues[0]);
-                else
-                {
+                if(val == NULL){
                     slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name, "agmtlist_modify_callback: " 
                                 "no value provided for %s attribute\n", type_nsds5ReplicaInitialize);
                     *returncode = LDAP_UNWILLING_TO_PERFORM;
@@ -524,19 +522,23 @@ agmtlist_modify_callback(Slapi_PBlock *pb, Slapi_Entry *entryBefore, Slapi_Entry
             }
         }
         else if (slapi_attr_types_equivalent(mods[i]->mod_type, type_replicaProtocolTimeout)){
-            if (val){
-                long ptimeout = atol(val);
+            long ptimeout = 0;
 
-                if(ptimeout <= 0){
-                    *returncode = LDAP_UNWILLING_TO_PERFORM;
-                    slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "attribute %s value (%s) is invalid, "
-                                    "must be a number greater than zero.\n",
-                                    type_replicaProtocolTimeout, val);
-                    rc = SLAPI_DSE_CALLBACK_ERROR;
-                    break;
-                }
-                agmt_set_protocol_timeout(agmt, ptimeout);
+            if (val){
+                ptimeout = atol(val);
+            }
+            if(ptimeout <= 0){
+                *returncode = LDAP_UNWILLING_TO_PERFORM;
+                PR_snprintf (returntext, SLAPI_DSE_RETURNTEXT_SIZE,
+                             "attribute %s value (%s) is invalid, must be a number greater than zero.\n",
+                             type_replicaProtocolTimeout, val ? val : "");
+                slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "attribute %s value (%s) is invalid, "
+                                "must be a number greater than zero.\n",
+                                type_replicaProtocolTimeout, val ? val : "");
+                rc = SLAPI_DSE_CALLBACK_ERROR;
+                break;
             }
+            agmt_set_protocol_timeout(agmt, ptimeout);
         }
         else if (0 == windows_handle_modify_agreement(agmt, mods[i]->mod_type, e))
         {
diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c
index 4a3f29f..9abbbac 100644
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
@@ -498,17 +498,21 @@ replica_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry*
                 else if (strcasecmp (config_attr, type_replicaProtocolTimeout) == 0 ){
                     if (apply_mods && config_attr_value && config_attr_value[0])
                     {
-                        long ptimeout = atol(config_attr_value);
+                        long ptimeout = 0;
+
+                        if(config_attr_value){
+                            ptimeout = atol(config_attr_value);
+                        }
 
                         if(ptimeout <= 0){
                             *returncode = LDAP_UNWILLING_TO_PERFORM;
                             PR_snprintf (errortext, SLAPI_DSE_RETURNTEXT_SIZE,
                                          "attribute %s value (%s) is invalid, must be a number greater than zero.\n",
-                                         config_attr, config_attr_value);
+                                         config_attr, config_attr_value ? config_attr_value : "");
                             slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "replica_config_modify: %s\n", errortext);
-                        } else {
-                            replica_set_protocol_timeout(r, ptimeout);
+                            break;
                         }
+                        replica_set_protocol_timeout(r, ptimeout);
                     }
                 }
                 else