Copilot commented on code in PR #14009:
URL: https://github.com/apache/cloudstack/pull/14009#discussion_r3887064605
##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -289,7 +289,9 @@ mount_operation() {
if [ ${NAS_TYPE} == "cifs" ]; then
MOUNT_OPTS="${MOUNT_OPTS},nobrl"
fi
- mount -t ${NAS_TYPE} ${NAS_ADDRESS} ${mount_point} $([[ ! -z "${MOUNT_OPTS}"
]] && echo -o ${MOUNT_OPTS}) 2>&1 | tee -a "$logFile"
+ 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"
Review Comment:
`mount_args` is introduced as a new global variable (arrays are global by
default in bash). Making it local to `mount_operation` avoids accidental
reuse/clobbering elsewhere in the script and keeps the function self-contained.
##########
api/src/main/java/org/apache/cloudstack/api/ApiArgValidator.java:
##########
@@ -79,14 +85,19 @@ public void validate(final Object paramObj, final Parameter
annotation) {
}
}
- private static void validateSafeCommandOptions(final Object param, final
String argName) {
+ private static void validateSafeMountCommandOptions(final Object param,
final String argName) {
+ if (param == null) {
+ return;
+ }
final String value = String.valueOf(param);
if (StringUtils.isBlank(value)) {
return;
}
Review Comment:
`validateSafeMountCommandOptions` returns early for
`StringUtils.isBlank(value)`, which means an all-whitespace mountOptions value
(e.g. " ") is accepted and bypasses the intended whitespace rejection. This
contradicts the goal of disallowing whitespace in mount options and can allow
whitespace-only inputs through validation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
