Ori Liel has uploaded a new change for review. Change subject: restapi: 'async' URL Parameter Malfunction (#957452) ......................................................................
restapi: 'async' URL Parameter Malfunction (#957452) If 'async' param appears in URL, then, regardless of the value assigned to it (true/false), the application would, erroniously, always consider it true. After the change, 'async=true', 'async' are considered true, 'async=false' or the absence of this parameter are considered false, and an illegal value ('async=blabla') throws exception. Change-Id: I8b8ac5d49823612300f1c7872c8d317a20e02797 Bug-Url: http://bugzilla.redhat.com/957452 Signed-off-by: Ori Liel <ol...@redhat.com> --- M backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/QueryHelper.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java 4 files changed, 36 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/94/15094/1 diff --git a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/QueryHelper.java b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/QueryHelper.java index 5ae0c55..acb43d9 100644 --- a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/QueryHelper.java +++ b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/QueryHelper.java @@ -185,4 +185,9 @@ } return params; } + + public static String getMatrixConstraint(UriInfo uriInfo, String constraint) { + return getMatrixConstraints(uriInfo, constraint).containsKey(constraint) ? + getMatrixConstraints(uriInfo, constraint).get(constraint) : null; + } } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java index 973e530..6644080 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java @@ -377,10 +377,6 @@ throw new WebApplicationException(Response.Status.NOT_FOUND); } - protected void badRequest(String message) { - throw new WebFaultException(null, message, Response.Status.BAD_REQUEST); - } - protected abstract class EntityIdResolver<T> implements IResolver<T, Q> { public abstract Q lookupEntity(T id) throws BackendFailureException; diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java index 53bcc99..4ce9dc8 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java @@ -32,9 +32,12 @@ import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil; +import org.ovirt.engine.api.restapi.resource.exception.UrlParamException; import org.ovirt.engine.api.restapi.resource.validation.Validator; public class BackendResource extends BaseBackendResource { + private static final String FALSE = "false"; + private static final String TRUE = "true"; protected static final int NO_LIMIT = -1; private static final String CORRELATION_ID = "Correlation-Id"; private static final String ASYNC_CONSTRAINT = "async"; @@ -145,8 +148,7 @@ protected Response performAction(VdcActionType task, VdcActionParametersBase params, Action action, boolean getEntityWhenDone) { try { - if (QueryHelper.hasMatrixParam(getUriInfo(), ASYNC_CONSTRAINT) || - expectNonBlocking()) { + if (isAsync() || expectNonBlocking()) { getCurrent().get(MetaData.class).set("async", true); return performNonBlockingAction(task, params, action); } else { @@ -165,6 +167,28 @@ } } + protected boolean isAsync() { + if (QueryHelper.hasMatrixParam(getUriInfo(), ASYNC_CONSTRAINT)) { + String value = QueryHelper.getMatrixConstraint(getUriInfo(), ASYNC_CONSTRAINT); + if (value.equalsIgnoreCase(TRUE) || value.isEmpty()) { + return true; + } else if (value.equalsIgnoreCase(FALSE)) { + return false; + } else { + // 'async' param exists but has illegal value (not 'false' or 'true') - throw exception. + throw new UrlParamException(this, + null, + "Illegal value '" + value + + "' for matrix param: 'async'. Acceptable values are 'true' or 'false'"); + } + } + return false; // matrix param does not exist in URL, go with the default value, which is 'false'. + } + + protected void badRequest(String message) { + throw new WebFaultException(null, message, Response.Status.BAD_REQUEST); + } + protected boolean expectNonBlocking() { boolean expectNonBlocking = false; List<String> expect = httpHeaders.getRequestHeader(EXPECT_HEADER); diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java index ecd9e63..930e17d 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java @@ -20,6 +20,7 @@ import org.ovirt.engine.api.model.Fault; import org.ovirt.engine.api.restapi.logging.MessageBundle; import org.ovirt.engine.api.restapi.logging.Messages; +import org.ovirt.engine.api.restapi.resource.exception.UrlParamException; import org.ovirt.engine.api.restapi.resource.validation.ValidatorLocator; import org.ovirt.engine.api.restapi.util.SessionHelper; import org.ovirt.engine.api.restapi.utils.MalformedIdException; @@ -157,7 +158,7 @@ } } - protected class WebFaultException extends WebApplicationException { + public class WebFaultException extends WebApplicationException { private static final long serialVersionUID = 394735369823915802L; private Fault fault; @@ -218,6 +219,9 @@ } else if ((e instanceof BackendFailureException) && (!StringUtils.isEmpty(e.getMessage()))) { LOG.errorFormat(localize(Messages.BACKEND_FAILED_TEMPLATE), e.getMessage(), null); throw new WebFaultException(null, e.getMessage(), Response.Status.BAD_REQUEST); + } else if (e instanceof UrlParamException){ + UrlParamException e2 = (UrlParamException) e; + throw e2; } else { LOG.errorFormat(localize(Messages.BACKEND_FAILED_TEMPLATE), detail(e), e); throw new WebFaultException(e, detail(e), Response.Status.INTERNAL_SERVER_ERROR); -- To view, visit http://gerrit.ovirt.org/15094 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8b8ac5d49823612300f1c7872c8d317a20e02797 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <ol...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches