api,kvm: reject a mount option list that only looks empty

Validation returned early for a blank value, so a list of nothing but whitespace
was accepted although whitespace is rejected everywhere else in the list. The
two sides then disagreed about it: the restore wrapper treats it as no options
at all, while the backup script sees a non empty string and hands it to mount as
an option of its own. An empty value still clears the options, anything else is
now validated.

The array holding the mount command is also declared local, as arrays assigned
in a bash function are otherwise global. mount_point and dest stay as they are,
the callers of mount_operation read them.

Author: abh1sar

api/src/main/java/org/apache/cloudstack/api/ApiArgValidator.java

@@ -90,7 +90,10 @@ private static void validateSafeMountCommandOptions(final Object param, final St
             return;
         }
         final String value = String.valueOf(param);
-        if (StringUtils.isBlank(value)) {
+        // An empty value clears the mount options and is allowed. A value that only looks empty is
+        // not: whitespace is rejected everywhere else in the list, and the backup script would pass
+        // it on to mount as an option of its own.
+        if (value.isEmpty()) {
             return;
         }
 

scripts/vm/hypervisor/kvm/nasbackup.sh

@@ -289,7 +289,7 @@ mount_operation() {
   if [ ${NAS_TYPE} == "cifs" ]; then
     MOUNT_OPTS="${MOUNT_OPTS},nobrl"
   fi
-  mount_args=(-t "${NAS_TYPE}" "${NAS_ADDRESS}" "${mount_point}")
+  local mount_args=(-t "${NAS_TYPE}" "${NAS_ADDRESS}" "${mount_point}")
   [[ -n "${MOUNT_OPTS}" ]] && mount_args+=(-o "${MOUNT_OPTS}")
   mount "${mount_args[@]}" 2>&1 | tee -a "$logFile"
   if [ $? -eq 0 ]; then

server/src/test/java/com/cloud/api/dispatch/ParamProcessWorkerTest.java

@@ -212,6 +212,24 @@ public void processMountOptionsParameter_RejectCephFsSecretWithCommandSubstituti
         paramProcessWorkerSpy.processParameters(cmd, params);
     }
 
+    @Test
+    public void processMountOptionsParameter_AcceptsEmptyValueToClearTheOptions() {
+        final HashMap<String, String> params = new HashMap<String, String>();
+        params.put("mountOptions", "");
+        final TestCmd cmd = new TestCmd();
+        paramProcessWorkerSpy.processParameters(cmd, params);
+        Assert.assertEquals("", cmd.mountOptions);
+    }
+
+    @Test(expected = ServerApiException.class)
+    public void processMountOptionsParameter_RejectWhitespaceOnly() {
+        final HashMap<String, String> params = new HashMap<String, String>();
+        // Only looks empty: the backup script would hand this to mount as an option.
+        params.put("mountOptions", "   ");
+        final TestCmd cmd = new TestCmd();
+        paramProcessWorkerSpy.processParameters(cmd, params);
+    }
+
     @Test(expected = ServerApiException.class)
     public void processMountOptionsParameter_RejectWhitespace() {
         final HashMap<String, String> params = new HashMap<String, String>();