api,server: normalize string empty value on config update

Fixes #10823

Signed-off-by: Abhishek Kumar <[email protected]>

Author: shwstppr

api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java

@@ -150,7 +150,7 @@ public void execute() {
             ConfigurationResponse response = _responseGenerator.createConfigurationResponse(cfg);
             response.setResponseName(getCommandName());
             response = setResponseScopes(response);
-            response = setResponseValue(response, cfg);
+            setResponseValue(response, cfg);
             this.setResponseObject(response);
         } else {
             throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update config");
@@ -161,15 +161,13 @@ public void execute() {
      * Sets the configuration value in the response. If the configuration is in the `Hidden` or `Secure` categories, the value is encrypted before being set in the response.
      * @param response to be set with the configuration `cfg` value
      * @param cfg to be used in setting the response value
-     * @return the response with the configuration's value
      */
-    public ConfigurationResponse setResponseValue(ConfigurationResponse response, Configuration cfg) {
+    public void setResponseValue(ConfigurationResponse response, Configuration cfg) {
+        String value = cfg.getValue();
         if (cfg.isEncrypted()) {
-            response.setValue(DBEncryptionUtil.encrypt(getValue()));
-        } else {
-            response.setValue(getValue());
+            value = DBEncryptionUtil.encrypt(value);
         }
-        return response;
+        response.setValue(value);
     }
 
     /**

api/src/test/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmdTest.java

@@ -0,0 +1,65 @@
+package org.apache.cloudstack.api.command.admin.config;
+
+import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.config.Configuration;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import com.cloud.utils.crypt.DBEncryptionUtil;
+
+@RunWith(MockitoJUnitRunner.class)
+public class UpdateCfgCmdTest {
+
+    private UpdateCfgCmd updateCfgCmd;
+
+    private MockedStatic<DBEncryptionUtil> mockedStatic;
+
+    @Before
+    public void setUp() {
+        updateCfgCmd = new UpdateCfgCmd();
+        mockedStatic = Mockito.mockStatic(DBEncryptionUtil.class);
+    }
+
+    @After
+    public void tearDown() {
+        mockedStatic.close();
+    }
+
+    @Test
+    public void setResponseValueSetsEncryptedValueWhenConfigurationIsEncrypted() {
+        ConfigurationResponse response = new ConfigurationResponse();
+        Configuration cfg = Mockito.mock(Configuration.class);
+        Mockito.when(cfg.isEncrypted()).thenReturn(true);
+        Mockito.when(cfg.getValue()).thenReturn("testValue");
+        Mockito.when(DBEncryptionUtil.encrypt("testValue")).thenReturn("encryptedValue");
+        updateCfgCmd.setResponseValue(response, cfg);
+        Assert.assertEquals("encryptedValue", response.getValue());
+    }
+
+    @Test
+    public void setResponseValueSetsPlainValueWhenConfigurationIsNotEncrypted() {
+        ConfigurationResponse response = new ConfigurationResponse();
+        Configuration cfg = Mockito.mock(Configuration.class);
+        Mockito.when(cfg.isEncrypted()).thenReturn(false);
+        Mockito.when(cfg.getValue()).thenReturn("testValue");
+        updateCfgCmd.setResponseValue(response, cfg);
+        Assert.assertEquals("testValue", response.getValue());
+    }
+
+    @Test
+    public void setResponseValueHandlesNullConfigurationValueGracefully() {
+        ConfigurationResponse response = new ConfigurationResponse();
+        Configuration cfg = Mockito.mock(Configuration.class);
+        Mockito.when(cfg.isEncrypted()).thenReturn(false);
+        Mockito.when(cfg.getValue()).thenReturn(null);
+        updateCfgCmd.setResponseValue(response, cfg);
+        Assert.assertNull(response.getValue());
+    }
+
+}

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -982,6 +982,19 @@ private void updateCustomDisplayNameOnHypervisorsList(String previousValue, Stri
         }
     }
 
+    protected String getNormalizedEmptyValueForConfig(final String name, final String inputValue,
+                          final Long configStorageId) {
+        String value = inputValue.trim();
+        if (!value.isEmpty() && !value.equals("null")) {
+            return value;
+        }
+        if (configStorageId != null) {
+            return "";
+        }
+        ConfigKey<?> key = _configDepot.get(name);
+        return (key != null && key.type() == String.class) ? "" : null;
+    }
+
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "updating configuration")
     public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidParameterValueException {
@@ -1076,11 +1089,7 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
             throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
         }
 
-        value = value.trim();
-
-        if (value.isEmpty() || value.equals("null")) {
-            value = (id == null) ? null : "";
-        }
+        value = getNormalizedEmptyValueForConfig(name, value, id);
 
         String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
         final String updatedValue = updateConfiguration(userId, name, category, value, scope, id);

server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java

@@ -1072,4 +1072,42 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenDetailsMatch
 
         Assert.assertFalse(result);
     }
+
+    @Test
+    public void normalizedEmptyValueForConfigReturnsTrimmedValueWhenInputIsValid() {
+        String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "  validValue  ", null);
+        Assert.assertEquals("validValue", result);
+    }
+
+    @Test
+    public void normalizedEmptyValueForConfigReturnsNullWhenInputIsNullAndNoConfigStorageId() {
+        String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "null", null);
+        Assert.assertNull(result);
+    }
+
+    @Test
+    public void normalizedEmptyValueForConfigReturnsEmptyStringWhenInputIsNullAndConfigStorageIdProvided() {
+        String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "null", 123L);
+        Assert.assertEquals("", result);
+    }
+
+    @Test
+    public void normalizedEmptyValueForConfigReturnsEmptyStringWhenKeyTypeIsStringAndInputIsEmpty() {
+        ConfigKey<String> mockKey = Mockito.mock(ConfigKey.class);
+        Mockito.when(mockKey.type()).thenReturn(String.class);
+        Mockito.doReturn(mockKey).when(configDepot).get("someConfig");
+
+        String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "", null);
+        Assert.assertEquals("", result);
+    }
+
+    @Test
+    public void normalizedEmptyValueForConfigReturnsNullWhenKeyTypeIsNotStringAndInputIsEmpty() {
+        ConfigKey<Integer> mockKey = Mockito.mock(ConfigKey.class);
+        Mockito.when(mockKey.type()).thenReturn(Integer.class);
+        Mockito.doReturn(mockKey).when(configDepot).get("someConfig");
+
+        String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "", null);
+        Assert.assertNull(result);
+    }
 }