You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by va...@apache.org on 2023/11/16 00:52:34 UTC
(tinkerpop) branch master updated: [TINKERPOP-2877] Added integer overflow checks (#2344)
This is an automated email from the ASF dual-hosted git repository.
valentyn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/master by this push:
new 846761004e [TINKERPOP-2877] Added integer overflow checks (#2344)
846761004e is described below
commit 846761004eed2b26cb79dcc9aa05f9d0f0f34769
Author: Valentyn Kahamlyk <vk...@users.noreply.github.com>
AuthorDate: Wed Nov 15 16:52:27 2023 -0800
[TINKERPOP-2877] Added integer overflow checks (#2344)
---
CHANGELOG.asciidoc | 2 ++
docs/src/upgrade/release-4.x.x.asciidoc | 4 +++
.../gremlin/process/traversal/Operator.java | 2 +-
.../tinkerpop/gremlin/util/NumberHelper.java | 36 ++++++++++++++-------
.../tinkerpop/gremlin/util/NumberHelperTest.java | 37 ++++++++++++++++++++++
5 files changed, 68 insertions(+), 13 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 2cb8d714cc..c711560485 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -24,6 +24,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
=== TinkerPop 4.0.0 (NOT OFFICIALLY RELEASED YET)
* Added support for deserialization of `Set` for `gremlin-javascript`.
+* Added integer overflow checks.
== TinkerPop 3.7.0 (Gremfir Master of the Pan Flute)
@@ -471,6 +472,7 @@ This release also includes changes from <<release-3-5-3, 3.5.3>>.
* Added `TextP.regex` and `TextP.notRegex`.
* Changed TinkerGraph to allow identifiers to be heterogeneous when filtering.
* Prevented values of `T` to `property()` from being `null`.
+* Added throwing `ArithmeticException` when arithmetic operations overflow for byte, short, int and long arguments.
* Added `element()` step.
* Added `call()` step.
* Added `fail()` step.
diff --git a/docs/src/upgrade/release-4.x.x.asciidoc b/docs/src/upgrade/release-4.x.x.asciidoc
index 4a922dbcc9..793c0f28a2 100644
--- a/docs/src/upgrade/release-4.x.x.asciidoc
+++ b/docs/src/upgrade/release-4.x.x.asciidoc
@@ -33,6 +33,10 @@ complete list of all the modifications that are part of this release.
Starting from this version, `gremlin-javascript` will deserialize `Set` data into a ECMAScript 2015 Set. Previously,
these were deserialized into arrays.
+==== Improved handling of integer overflows
+Integer overflows caused by addition and multiplication operations will throw an exception instead of being silently
+skipped with incorrect result.
+
=== Upgrading for Providers
==== Graph System Providers
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java
index b4d3cb23a0..5bb2c85b3a 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java
@@ -202,7 +202,7 @@ public enum Operator implements BinaryOperator<Object> {
*/
sumLong {
public Object apply(final Object a, final Object b) {
- return (long) a + (long) b;
+ return NumberHelper.add((long) a, (long) b);
}
}
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java
index e4a2da8281..2d1e6154ad 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java
@@ -28,10 +28,22 @@ import java.util.function.BiFunction;
*/
public final class NumberHelper {
+ private static byte asByte(int arg) {
+ if (arg > Byte.MAX_VALUE || arg < Byte.MIN_VALUE)
+ throw new ArithmeticException("byte overflow");
+ return (byte) arg;
+ }
+
+ private static short asShort(int arg) {
+ if (arg > Short.MAX_VALUE || arg < Short.MIN_VALUE)
+ throw new ArithmeticException("short overflow");
+ return (short) arg;
+ }
+
static final NumberHelper BYTE_NUMBER_HELPER = new NumberHelper(
- (a, b) -> (byte) (a.byteValue() + b.byteValue()),
- (a, b) -> (byte) (a.byteValue() - b.byteValue()),
- (a, b) -> (byte) (a.byteValue() * b.byteValue()),
+ (a, b) -> asByte(a.byteValue() + b.byteValue()),
+ (a, b) -> asByte(a.byteValue() - b.byteValue()),
+ (a, b) -> asByte(a.byteValue() * b.byteValue()),
(a, b) -> (byte) (a.byteValue() / b.byteValue()),
(a, b) -> {
if (isNumber(a)) {
@@ -56,9 +68,9 @@ public final class NumberHelper {
(a, b) -> Byte.compare(a.byteValue(), b.byteValue()));
static final NumberHelper SHORT_NUMBER_HELPER = new NumberHelper(
- (a, b) -> (short) (a.shortValue() + b.shortValue()),
- (a, b) -> (short) (a.shortValue() - b.shortValue()),
- (a, b) -> (short) (a.shortValue() * b.shortValue()),
+ (a, b) -> asShort(a.shortValue() + b.shortValue()),
+ (a, b) -> asShort(a.shortValue() - b.shortValue()),
+ (a, b) -> asShort(a.shortValue() * b.shortValue()),
(a, b) -> (short) (a.shortValue() / b.shortValue()),
(a, b) -> {
if (isNumber(a)) {
@@ -83,9 +95,9 @@ public final class NumberHelper {
(a, b) -> Short.compare(a.shortValue(), b.shortValue()));
static final NumberHelper INTEGER_NUMBER_HELPER = new NumberHelper(
- (a, b) -> a.intValue() + b.intValue(),
- (a, b) -> a.intValue() - b.intValue(),
- (a, b) -> a.intValue() * b.intValue(),
+ (a, b) -> Math.addExact(a.intValue(), b.intValue()),
+ (a, b) -> Math.subtractExact(a.intValue(), b.intValue()),
+ (a, b) -> Math.multiplyExact(a.intValue(), b.intValue()),
(a, b) -> a.intValue() / b.intValue(),
(a, b) -> {
if (isNumber(a)) {
@@ -110,9 +122,9 @@ public final class NumberHelper {
(a, b) -> Integer.compare(a.intValue(), b.intValue()));
static final NumberHelper LONG_NUMBER_HELPER = new NumberHelper(
- (a, b) -> a.longValue() + b.longValue(),
- (a, b) -> a.longValue() - b.longValue(),
- (a, b) -> a.longValue() * b.longValue(),
+ (a, b) -> Math.addExact(a.longValue(), b.longValue()),
+ (a, b) -> Math.subtractExact(a.longValue(), b.longValue()),
+ (a, b) -> Math.multiplyExact(a.longValue(), b.longValue()),
(a, b) -> a.longValue() / b.longValue(),
(a, b) -> {
if (isNumber(a)) {
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/NumberHelperTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/NumberHelperTest.java
index 5b32e1981b..d8aed24bea 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/NumberHelperTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/NumberHelperTest.java
@@ -19,6 +19,7 @@
package org.apache.tinkerpop.gremlin.util;
import org.javatuples.Quartet;
+import org.javatuples.Triplet;
import org.junit.Test;
import java.math.BigDecimal;
@@ -38,6 +39,7 @@ import static org.apache.tinkerpop.gremlin.util.NumberHelper.sub;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
/**
* @author Daniel Kuppitz (http://gremlin.guru)
@@ -96,6 +98,21 @@ public class NumberHelperTest {
new Quartet<>(BigDecimal.ONE, BigDecimal.ONE, BigDecimal.class, BigDecimal.class)
);
+ private final static List<Triplet<Number, Number, String>> OVERFLOW_CASES = Arrays.asList(
+ new Triplet<>(Integer.MAX_VALUE, 1, "add"),
+ new Triplet<>(Integer.MIN_VALUE, 1, "sub"),
+ new Triplet<>(Integer.MAX_VALUE, Integer.MAX_VALUE, "mul"),
+ new Triplet<>(Long.MAX_VALUE, 1L, "add"),
+ new Triplet<>(Long.MIN_VALUE, 1L, "sub"),
+ new Triplet<>(Long.MAX_VALUE, Integer.MAX_VALUE, "mul"),
+ new Triplet<>(Byte.MAX_VALUE, (byte)100, "add"),
+ new Triplet<>(Byte.MIN_VALUE, (byte)100, "sub"),
+ new Triplet<>((byte)100, (byte)100, "mul"),
+ new Triplet<>(Short.MAX_VALUE, (short)100, "add"),
+ new Triplet<>(Short.MIN_VALUE, (short)100, "sub"),
+ new Triplet<>(Short.MAX_VALUE, (short)100, "mul")
+ );
+
@Test
public void shouldReturnHighestCommonNumberClass() {
for (final Quartet<Number, Number, Class<? extends Number>, Class<? extends Number>> q : COMMON_NUMBER_CLASSES) {
@@ -442,4 +459,24 @@ public class NumberHelperTest {
assertEquals(1, compare(one, null).intValue());
}
}
+
+ @Test
+ public void shouldThrowArithmeticExceptionOnOverflow() {
+ for (final Triplet<Number, Number, String> q : OVERFLOW_CASES) {
+ try {
+ switch (q.getValue2()) {
+ case "add":
+ add(q.getValue0(), q.getValue1());
+ case "sub":
+ sub(q.getValue0(), q.getValue1());
+ case "mul":
+ mul(q.getValue0(), q.getValue1());
+ }
+ fail("ArithmeticException expected");
+ }
+ catch (ArithmeticException ex) {
+ // expected
+ }
+ }
+ }
}