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, 168 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/56/23856/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..67f8a20 --- /dev/null +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/IntegerParser.java @@ -0,0 +1,29 @@ +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 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 Integer.MAX_VALUE"); + } + return result.intValue(); + } + + public static String printIntToInteger(int value) { + return DatatypeConverter.printInteger(BigInteger.valueOf(value)); + } +} 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 46aa30f..05f821a 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"> + <jaxb:javaType name="int" xmlType="xs:int" + parseMethod="org.ovirt.engine.api.utils.IntegerParser.parseIntegerToInt" + printMethod="org.ovirt.engine.api.utils.IntegerParser.printIntToInteger" + /> + <jaxb:javaType name="int" xmlType="xs:unsignedShort" + parseMethod="org.ovirt.engine.api.utils.IntegerParser.parseIntegerToShort" + printMethod="org.ovirt.engine.api.utils.IntegerParser.printIntToInteger" + /> + </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 8a29d6a..3fa9396 100644 --- a/backend/manager/modules/restapi/jaxrs/pom.xml +++ b/backend/manager/modules/restapi/jaxrs/pom.xml @@ -107,6 +107,11 @@ <type>test-jar</type> <scope>test</scope> </dependency> + <dependency> + <groupId>org.jboss.resteasy</groupId> + <artifactId>resteasy-jaxb-provider</artifactId> + <version>2.3.2.Final</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..ed0036c 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; @@ -161,11 +163,13 @@ singletons.add(new RequestPayloadLogger()); singletons.add(new ResponseStatusLogger()); singletons.add(new ResponsePayloadLogger()); + 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..fcc7a28 --- /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 javax.ws.rs.core.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..7645db3 --- /dev/null +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/XmlMessageBodyReader.java @@ -0,0 +1,84 @@ +package org.ovirt.engine.api.restapi.resource.validation; + +import org.jboss.resteasy.plugins.providers.jaxb.AbstractJAXBProvider; +import org.jboss.resteasy.plugins.providers.jaxb.JAXBContextFinder; +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.Context; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.ext.ContextResolver; +import javax.ws.rs.ext.MessageBodyReader; +import javax.ws.rs.ext.Provider; +import javax.ws.rs.ext.Providers; +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 implements MessageBodyReader<Object> { + + /** XML parsers */ + @Context + protected Providers providers; + + /** Default event handler recognizes XML parsing as error and not as warning */ + private ValidationEventHandler errorhandler = new DefaultValidationEventHandler(); + + protected JAXBContextFinder getFinder(MediaType type) { + ContextResolver<JAXBContextFinder> resolver = providers.getContextResolver(JAXBContextFinder.class, type); + if (resolver == null) { + throw new IllegalArgumentException("Unable to find ContextResolver for media type: " + type); + } + return resolver.getContext(null); + } + + /** {@inheritDoc} */ + @Override + public boolean isReadable(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 = getFinder(mediaType).findCacheContext(mediaType, annotations, type); + 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/23856 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9d0be6f3f220d7a5f58bb47906a8565d80e0ebf0 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
