Hello Juan Hernandez,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/23950
to review the following change.
Change subject: restapi: Validate unsigned short integers correctly
......................................................................
restapi: Validate unsigned short integers correctly
Recently, in commit 14e9da, we introduced a validation of integer values
provided to the RESTAPI in order to avoid overflows of Java integer
types. This validation doesn't work correctly for unsigned short
integers, as it compares them to the maximum signed short integer. This
has the side effect of rejecting requests that include port numbers
greater than 32767. A notable example is the VDSM port number 54321.
This patch changes the validation so that it will validate unsigned
short integers correctly, comparing them to 65535.
Change-Id: I49e64d5f55c32463950e648aa1cd8c3084017da6
Signed-off-by: Juan Hernandez <juan.hernan...@redhat.com>
---
M
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/IntegerParser.java
M
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
2 files changed, 54 insertions(+), 18 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/50/23950/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
index 72e6032..f9c5d4d 100644
---
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
@@ -3,24 +3,46 @@
import javax.xml.bind.DatatypeConverter;
import java.math.BigInteger;
+/**
+ * This class contains methods that parse integers from strings checking and
avoiding overflows of the corresponding
+ * Java integer types.
+ */
public class IntegerParser {
+ // Max values of Java integer types:
+ private static final BigInteger MAX_SHORT = new BigInteger("32767"); //
2^(16-1)-1
+ private static final BigInteger MAX_UNSIGNED_SHORT = new
BigInteger("65535"); // 2^16-1
+ private static final BigInteger MAX_INT = new BigInteger("2147483647"); //
2^(32-1)-1
+ private static final BigInteger MAX_UNSIGNED_INT = new
BigInteger("4294967295"); // 2^32-1
- 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);
+ public static short parseShort(String value) {
+ BigInteger result = DatatypeConverter.parseInteger(value);
+ if (result.compareTo(MAX_SHORT) > 0) {
+ throw new InvalidValueException("Value " + value + " is greater
than the maximum short " + MAX_SHORT);
+ }
+ return result.shortValue();
+ }
+
+ public static int parseUnsignedShort(String value) {
+ BigInteger result = DatatypeConverter.parseInteger(value);
+ if (result.compareTo(MAX_UNSIGNED_SHORT) > 0) {
+ throw new InvalidValueException("Value " + value + " is greater
than maximum unsigned short " + MAX_UNSIGNED_SHORT);
}
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);
+ public static int parseInt(String value) {
+ BigInteger result = DatatypeConverter.parseInteger(value);
+ if (result.compareTo(MAX_INT) > 0) {
+ throw new InvalidValueException("Value " + value + " is greater
than maximum integer " + MAX_INT);
}
return result.intValue();
}
+ public static long parseUnsignedInt(String value) {
+ BigInteger result = DatatypeConverter.parseInteger(value);
+ if (result.compareTo(MAX_UNSIGNED_INT) > 0) {
+ throw new InvalidValueException("Value " + value + " is greater
than maximum unsigned integer " + MAX_UNSIGNED_INT);
+ }
+ return result.longValue();
+ }
}
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 0cd4d60..a20041e 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
@@ -8,14 +8,28 @@
<xs:annotation>
<xs:appinfo>
<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"
- />
+ <!-- Replace the default int JAXB parser with one that checks the
values don't overflow the corresponding
+ Java types: -->
+ <jaxb:javaType
+ name="int"
+ xmlType="xs:int"
+ parseMethod="org.ovirt.engine.api.utils.IntegerParser.parseInt"
+ />
+ <jaxb:javaType
+ name="long"
+ xmlType="xs:unsignedInt"
+
parseMethod="org.ovirt.engine.api.utils.IntegerParser.parseUnsignedInt"
+ />
+ <jaxb:javaType
+ name="short"
+ xmlType="xs:short"
+ parseMethod="org.ovirt.engine.api.utils.IntegerParser.parseShort"
+ />
+ <jaxb:javaType
+ name="int"
+ xmlType="xs:unsignedShort"
+
parseMethod="org.ovirt.engine.api.utils.IntegerParser.parseUnsignedShort"
+ />
</jaxb:globalBindings>
</xs:appinfo>
</xs:annotation>
--
To view, visit http://gerrit.ovirt.org/23950
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I49e64d5f55c32463950e648aa1cd8c3084017da6
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
