Copilot commented on code in PR #11976:
URL: https://github.com/apache/cloudstack/pull/11976#discussion_r3568687039
##########
server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java:
##########
@@ -510,7 +511,7 @@ private Long translateUuidToInternalId(final String uuid,
final Parameter annota
}
Long internalId = null;
// If annotation's empty, the cmd existed before 3.x try conversion to
long
- final boolean isPre3x = annotation.since().isEmpty();
+ final boolean isPre3x = annotation.since().isEmpty() &&
!ApiServer.ApiDisallowInternalIds.value();
Review Comment:
With api.disallow.internal.ids enabled (default), non-UUID inputs for UUID
parameters currently hit the "!isPre3x && !isUuid" branch and return null. That
silently drops the provided parameter (and can broaden list results) instead of
returning a PARAM_ERROR, so internal DB IDs (and other invalid formats) are not
reliably rejected.
##########
server/src/main/java/com/cloud/api/ApiServer.java:
##########
@@ -348,6 +348,19 @@ public class ApiServer extends ManagerBase implements
HttpRequestHandler, ApiSer
, ConfigKey.Scope.Global, null, null, null, null, null,
ConfigKey.Kind.Select,
EnumSet.allOf(ApiSessionKeyCheckOption.class).stream().map(Enum::toString).collect(Collectors.joining(",
")));
+ public static final ConfigKey<Boolean> ApiDisallowInternalIds = new
ConfigKey<>(
+ ConfigKey.CATEGORY_ADVANCED,
+ Boolean.class,
+ "api.disallow.internal.ids",
+ "true",
+ "When enabled, APIs will not honour requests containing internal
database IDs. "
+ + "Only UUIDs will be accepted as entity identifiers. "
+ + "By default, internal IDs are not accepted even for
pre-3.x APIs.",
Review Comment:
The new global setting description is a bit ambiguous about the effect of
disabling it. Since this is a user-facing config description, it would be
clearer to explicitly document the default and what happens when set to false
(legacy pre-3.x APIs accepting internal IDs).
##########
server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java:
##########
@@ -510,7 +511,7 @@ private Long translateUuidToInternalId(final String uuid,
final Parameter annota
}
Long internalId = null;
// If annotation's empty, the cmd existed before 3.x try conversion to
long
- final boolean isPre3x = annotation.since().isEmpty();
+ final boolean isPre3x = annotation.since().isEmpty() &&
!ApiServer.ApiDisallowInternalIds.value();
Review Comment:
This change adds a new config-controlled behavior for UUID parsing, but
there are no unit tests covering that internal IDs / non-UUID inputs are
rejected when api.disallow.internal.ids is true (the default). Adding a focused
ParamProcessWorkerTest case would prevent regressions and confirm the intended
behavior from #9684.
--
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]
