Ravi Nori has uploaded a new change for review. Change subject: restapi : ints and shorts overflow ......................................................................
restapi : ints and shorts overflow int and short values are overflown before they even reach the business code, allowing for wrong values to be set. When posting to rest api, if the value of an int is greater than Integer.MAX_VALUE or value of a short is larger than Short.MAX_VALUE. The values are overflown when values are unmarshalled using JAXB. An excpetion should be raised during JAXB unmarshalling to indicate that the value is greater than the MAX_VALUE for the type. Change-Id: I9d0be6f3f220d7a5f58bb47906a8565d80e0ebf0 Bug-Url: https://bugzilla.redhat.com/1000796 Signed-off-by: Ravi Nori <rn...@redhat.com> --- A backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/IntegerParser.java A backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/InvalidValueException.java M backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd M backend/manager/modules/restapi/jaxrs/pom.xml M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/BackendApplication.java A backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/InvalidValueExceptionMapper.java A backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/XmlMessageBodyReader.java M backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/VmMapperTest.java 8 files changed, 151 insertions(+), 8 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/49/23949/1 diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/IntegerParser.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/IntegerParser.java new file mode 100644 index 0000000..72e6032 --- /dev/null +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/IntegerParser.java @@ -0,0 +1,26 @@ +package org.ovirt.engine.api.utils; + +import javax.xml.bind.DatatypeConverter; +import java.math.BigInteger; + +public class IntegerParser { + + public static int parseIntegerToShort(String value) { + BigInteger result = + DatatypeConverter.parseInteger(value); + if (result.longValue() > Short.MAX_VALUE) { + throw new InvalidValueException("Value " +value+ " greater than maximum " + Short.MAX_VALUE); + } + return result.intValue(); + } + + public static int parseIntegerToInt(String value) { + BigInteger result = + DatatypeConverter.parseInteger(value); + if (result.longValue() > Integer.MAX_VALUE) { + throw new InvalidValueException("Value " +value+ " greater than maximum " + Integer.MAX_VALUE); + } + return result.intValue(); + } + +} diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/InvalidValueException.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/InvalidValueException.java new file mode 100644 index 0000000..295940d --- /dev/null +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/InvalidValueException.java @@ -0,0 +1,9 @@ +package org.ovirt.engine.api.utils; + +public class InvalidValueException extends RuntimeException { + + public InvalidValueException(String msg) { + super(msg); + } + +} diff --git a/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd b/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd index 0a2f6da..0cd4d60 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd +++ b/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd @@ -4,9 +4,19 @@ xmlns:xjc="http://java.sun.com/xml/ns/jaxb/xjc"; jaxb:version="2.1" jaxb:extensionBindingPrefixes="xjc"> + <xs:annotation> <xs:appinfo> - <jaxb:globalBindings generateIsSetMethod="true"/> + <jaxb:globalBindings generateIsSetMethod="true"> + <!-- Replace the default int JAXB parser with one that checks the value is less than Integer.MAX_VALUE --> + <jaxb:javaType name="int" xmlType="xs:int" + parseMethod="org.ovirt.engine.api.utils.IntegerParser.parseIntegerToInt" + /> + <!-- Replace the default short JAXB parser with one that checks the value is less than Short.MAX_VALUE --> + <jaxb:javaType name="int" xmlType="xs:unsignedShort" + parseMethod="org.ovirt.engine.api.utils.IntegerParser.parseIntegerToShort" + /> + </jaxb:globalBindings> </xs:appinfo> </xs:annotation> diff --git a/backend/manager/modules/restapi/jaxrs/pom.xml b/backend/manager/modules/restapi/jaxrs/pom.xml index cfe0ed9..83259d8 100644 --- a/backend/manager/modules/restapi/jaxrs/pom.xml +++ b/backend/manager/modules/restapi/jaxrs/pom.xml @@ -100,13 +100,18 @@ <scope>test</scope> </dependency> - <dependency> + <dependency> <groupId>org.ovirt.engine.api</groupId> <artifactId>restapi-types</artifactId> <version>${project.version}</version> <type>test-jar</type> <scope>test</scope> </dependency> + <dependency> + <groupId>org.jboss.resteasy</groupId> + <artifactId>resteasy-jaxb-provider</artifactId> + <version>${resteasy.version}</version> + </dependency> </dependencies> diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/BackendApplication.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/BackendApplication.java index fb0a3fe..f630a23 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/BackendApplication.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/BackendApplication.java @@ -58,10 +58,12 @@ import org.ovirt.engine.api.restapi.resource.BackendVmPoolsResource; import org.ovirt.engine.api.restapi.resource.BackendVmsResource; import org.ovirt.engine.api.restapi.resource.BackendVnicProfilesResource; +import org.ovirt.engine.api.restapi.resource.validation.InvalidValueExceptionMapper; import org.ovirt.engine.api.restapi.resource.validation.JaxbExceptionMapper; import org.ovirt.engine.api.restapi.resource.validation.JsonExceptionMapper; import org.ovirt.engine.api.restapi.resource.validation.MalformedIdExceptionMapper; import org.ovirt.engine.api.restapi.resource.validation.ValidatorLocator; +import org.ovirt.engine.api.restapi.resource.validation.XmlMessageBodyReader; import org.ovirt.engine.api.restapi.security.auth.LoginValidator; import org.ovirt.engine.api.restapi.types.MappingLocator; import org.ovirt.engine.api.restapi.util.SessionHelper; @@ -162,10 +164,14 @@ singletons.add(new ResponseStatusLogger()); singletons.add(new ResponsePayloadLogger()); + // JAXB message body reader that sets default validation event handler + singletons.add(new XmlMessageBodyReader()); + // Intercepter that maps exceptions cause by illegal guid string to 400 status (BAD_REQUEST). singletons.add(new MalformedIdExceptionMapper()); singletons.add(new JaxbExceptionMapper()); singletons.add(new JsonExceptionMapper()); + singletons.add(new InvalidValueExceptionMapper()); } private void addResource(final BackendResource resource) { diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/InvalidValueExceptionMapper.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/InvalidValueExceptionMapper.java new file mode 100644 index 0000000..7b328ed --- /dev/null +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/InvalidValueExceptionMapper.java @@ -0,0 +1,20 @@ +package org.ovirt.engine.api.restapi.resource.validation; + +import org.ovirt.engine.api.model.Fault; +import org.ovirt.engine.api.utils.InvalidValueException; + +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; + +@Provider +public class InvalidValueExceptionMapper implements ExceptionMapper<InvalidValueException> { + + @Override + public Response toResponse(InvalidValueException e) { + Fault fault = new Fault(); + fault.setReason("Invalid Value"); + fault.setDetail(e.getMessage()); + return Response.status(Response.Status.BAD_REQUEST).entity(fault).build(); + } +} diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/XmlMessageBodyReader.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/XmlMessageBodyReader.java new file mode 100644 index 0000000..9d05f98 --- /dev/null +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/XmlMessageBodyReader.java @@ -0,0 +1,67 @@ +package org.ovirt.engine.api.restapi.resource.validation; + +import org.jboss.resteasy.plugins.providers.jaxb.AbstractJAXBProvider; +import org.jboss.resteasy.plugins.providers.jaxb.JAXBMarshalException; +import org.ovirt.engine.api.utils.InvalidValueException; + +import javax.ws.rs.Consumes; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.ext.Provider; +import javax.xml.bind.JAXBContext; +import javax.xml.bind.JAXBElement; +import javax.xml.bind.JAXBException; +import javax.xml.bind.Unmarshaller; +import javax.xml.bind.ValidationEventHandler; +import javax.xml.bind.helpers.DefaultValidationEventHandler; +import java.io.IOException; +import java.io.InputStream; +import java.lang.annotation.Annotation; +import java.lang.reflect.Type; + +@Provider +@Consumes({ MediaType.APPLICATION_XML }) +public class XmlMessageBodyReader extends AbstractJAXBProvider<Object> { + + /** Default event handler recognizes XML parsing as error and not as warning */ + private ValidationEventHandler errorhandler = new DefaultValidationEventHandler(); + + @Override + protected boolean isReadWritable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) { + return true; + } + + /** + * {@inheritDoc} + * <p> + * Standard JAXB unmarshaller with custom error handler- {@link #errorhandler} + */ + @Override + public Object readFrom(Class<Object> type, Type genericType, Annotation[] annotations, MediaType mediaType, + MultivaluedMap<String, String> httpHeaders, InputStream entityStream) throws IOException, WebApplicationException { + + if (entityStream == null) { + return null; + } + try { + JAXBContext ctx = findJAXBContext(type, annotations, mediaType, true); + Unmarshaller unmarshaller = ctx.createUnmarshaller(); + + AbstractJAXBProvider.decorateUnmarshaller(type, annotations, mediaType, unmarshaller); + + unmarshaller.setEventHandler(errorhandler); + Object parsedObj = unmarshaller.unmarshal(entityStream); + if (parsedObj instanceof JAXBElement) { + return ((JAXBElement) parsedObj).getValue(); + } + return parsedObj; + } catch (JAXBException e) { + if (e.getLinkedException().getCause() instanceof InvalidValueException) { + throw (InvalidValueException) e.getLinkedException().getCause(); + } + throw new JAXBMarshalException(e); + } + } + +} diff --git a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/VmMapperTest.java b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/VmMapperTest.java index 8bff7af..229e994 100644 --- a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/VmMapperTest.java +++ b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/VmMapperTest.java @@ -258,7 +258,7 @@ assertNotNull(cpuTune); assertNotNull(cpuTune.getVCpuPin()); assertEquals(1, cpuTune.getVCpuPin().size()); - assertEquals(0, cpuTune.getVCpuPin().get(0).getVcpu()); + assertEquals(0, (int) cpuTune.getVCpuPin().get(0).getVcpu()); assertEquals("0", cpuTune.getVCpuPin().get(0).getCpuSet()); } @@ -275,26 +275,26 @@ @Test() public void stringToVCpupinIntervalsList() { VCpuPin pin = VmMapper.stringToVCpupin("1#1-4,6"); - assertEquals(1, pin.getVcpu()); + assertEquals(1, (int) pin.getVcpu()); assertEquals("1-4,6", pin.getCpuSet()); } @Test() public void stringToVCpupinSimple() { VCpuPin pin = VmMapper.stringToVCpupin("1#1"); - assertEquals(1, pin.getVcpu()); + assertEquals(1, (int) pin.getVcpu()); assertEquals("1", pin.getCpuSet()); pin = VmMapper.stringToVCpupin("1#10"); - assertEquals(1, pin.getVcpu()); + assertEquals(1, (int) pin.getVcpu()); assertEquals("10", pin.getCpuSet()); pin = VmMapper.stringToVCpupin("1#10,11,12"); - assertEquals(1, pin.getVcpu()); + assertEquals(1, (int) pin.getVcpu()); assertEquals("10,11,12", pin.getCpuSet()); pin = VmMapper.stringToVCpupin("1#10-12,16"); - assertEquals(1, pin.getVcpu()); + assertEquals(1, (int) pin.getVcpu()); assertEquals("10-12,16", pin.getCpuSet()); } -- To view, visit http://gerrit.ovirt.org/23949 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9d0be6f3f220d7a5f58bb47906a8565d80e0ebf0 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Ravi Nori <rn...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
