Browse Source

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!)
Mark Reynolds 12 years ago
parent
commit
8a4bbc7c74

+ 17 - 15
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++)
 	for (i = 0; NULL != mods && NULL != mods[i]; i++)
 	{
 	{
 		slapi_ch_free_string(&val);
 		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))
 		if (slapi_attr_types_equivalent(mods[i]->mod_type, type_nsds5ReplicaInitialize))
 		{
 		{
             /* we don't allow delete attribute operations unless it was issued by
             /* 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
             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: " 
                     slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name, "agmtlist_modify_callback: " 
                                 "no value provided for %s attribute\n", type_nsds5ReplicaInitialize);
                                 "no value provided for %s attribute\n", type_nsds5ReplicaInitialize);
                     *returncode = LDAP_UNWILLING_TO_PERFORM;
                     *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)){
         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))
         else if (0 == windows_handle_modify_agreement(agmt, mods[i]->mod_type, e))
         {
         {

+ 8 - 4
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 ){
                 else if (strcasecmp (config_attr, type_replicaProtocolTimeout) == 0 ){
                     if (apply_mods && config_attr_value && config_attr_value[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){
                         if(ptimeout <= 0){
                             *returncode = LDAP_UNWILLING_TO_PERFORM;
                             *returncode = LDAP_UNWILLING_TO_PERFORM;
                             PR_snprintf (errortext, SLAPI_DSE_RETURNTEXT_SIZE,
                             PR_snprintf (errortext, SLAPI_DSE_RETURNTEXT_SIZE,
                                          "attribute %s value (%s) is invalid, must be a number greater than zero.\n",
                                          "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);
                             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
                 else