This is an automated email from the ASF dual-hosted git repository.
uros-b pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new 2c60d60e0a8d [SPARK-57597][CORE] Guard ByteUnit.toBytes against long
overflow
2c60d60e0a8d is described below
commit 2c60d60e0a8d8d9bbd01727799008a6ac9d7db18
Author: YangJie <[email protected]>
AuthorDate: Tue Jun 23 17:43:09 2026 +0200
[SPARK-57597][CORE] Guard ByteUnit.toBytes against long overflow
### What changes were proposed in this pull request?
`ByteUnit.toBytes` checked for a negative argument and then returned `d *
multiplier` directly, with no guard against the multiplication overflowing a
`long`. For the larger units this can wrap silently: `PiB.toBytes(8192)`
returns `Long.MIN_VALUE` instead of reporting an out-of-range value. The
sibling `convertTo` already guards the same multiplication, so this PR adds the
equivalent check to `toBytes`. The overflow check is shared by both methods
through a small private `checkedMult [...]
It also adds `ByteUnitSuite`, which the module did not have, covering the
per-unit conversions (including zero), negative input, the overflow case, and
the largest value that still fits in a `long`.
### Why are the changes needed?
A size conversion that silently wraps to a negative number is worse than
one that fails loudly, and `toBytes` was the only conversion method on this
enum without the overflow guard that `convertTo` already has. Sharing the check
through `checkedMultiply` removes the duplicated idiom and keeps the two
methods consistent.
### Does this PR introduce _any_ user-facing change?
No behavioral change. Values that fit in a `long` convert exactly as
before; only an input large enough to overflow now throws
`IllegalArgumentException` instead of returning a wrapped value. The only other
observable difference is the wording of that exception message, which is now
shared between `toBytes` and `convertTo`.
### How was this patch tested?
Added `ByteUnitSuite`. `build/sbt 'common-utils-java/testOnly
*ByteUnitSuite'` passes, and the existing `UtilsSuite` "Test byteString
conversion" (which exercises the `convertTo` overflow path) still passes.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
Closes #56655 from LuciferYang/SPARK-byteunit-tobytes-overflow.
Authored-by: YangJie <[email protected]>
Signed-off-by: Uros Bojanic <[email protected]>
(cherry picked from commit 01a7a0f0db10f6e242bcd1e62ccc72d5acd1f5ac)
Signed-off-by: Uros Bojanic <[email protected]>
---
.../org/apache/spark/network/util/ByteUnit.java | 19 ++++---
.../apache/spark/network/util/ByteUnitSuite.java | 59 ++++++++++++++++++++++
2 files changed, 72 insertions(+), 6 deletions(-)
diff --git
a/common/utils-java/src/main/java/org/apache/spark/network/util/ByteUnit.java
b/common/utils-java/src/main/java/org/apache/spark/network/util/ByteUnit.java
index 6f7925c26094..99ce708a0af3 100644
---
a/common/utils-java/src/main/java/org/apache/spark/network/util/ByteUnit.java
+++
b/common/utils-java/src/main/java/org/apache/spark/network/util/ByteUnit.java
@@ -38,11 +38,7 @@ public enum ByteUnit {
public long convertTo(long d, ByteUnit u) {
if (multiplier > u.multiplier) {
long ratio = multiplier / u.multiplier;
- if (Long.MAX_VALUE / ratio < d) {
- throw new IllegalArgumentException("Conversion of " + d + " exceeds
Long.MAX_VALUE in "
- + name() + ". Try a larger unit (e.g. MiB instead of KiB)");
- }
- return d * ratio;
+ return checkedMultiply(d, ratio);
} else {
// Perform operations in this order to avoid potential overflow
// when computing d * multiplier
@@ -54,7 +50,18 @@ public enum ByteUnit {
if (d < 0) {
throw new IllegalArgumentException("Negative size value. Size must be
positive: " + d);
}
- return d * multiplier;
+ return checkedMultiply(d, multiplier);
+ }
+
+ // Multiply `d` by `factor`, throwing IllegalArgumentException if the
product would overflow a
+ // long. Shared by the conversions that scale up to a smaller unit
(including toBytes), where
+ // `d * factor` can exceed Long.MAX_VALUE.
+ private long checkedMultiply(long d, long factor) {
+ if (Long.MAX_VALUE / factor < d) {
+ throw new IllegalArgumentException("Conversion of " + d + " " + name()
+ + " exceeds Long.MAX_VALUE. Try a larger unit (e.g. MiB instead of
KiB).");
+ }
+ return d * factor;
}
public long toKiB(long d) { return convertTo(d, KiB); }
diff --git
a/common/utils-java/src/test/java/org/apache/spark/network/util/ByteUnitSuite.java
b/common/utils-java/src/test/java/org/apache/spark/network/util/ByteUnitSuite.java
new file mode 100644
index 000000000000..3002fceed44e
--- /dev/null
+++
b/common/utils-java/src/test/java/org/apache/spark/network/util/ByteUnitSuite.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.network.util;
+
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
+class ByteUnitSuite {
+
+ @Test
+ void toBytesConvertsEachUnit() {
+ assertEquals(0L, ByteUnit.PiB.toBytes(0));
+ assertEquals(10L, ByteUnit.BYTE.toBytes(10));
+ assertEquals(1L << 10, ByteUnit.KiB.toBytes(1));
+ assertEquals(1L << 20, ByteUnit.MiB.toBytes(1));
+ assertEquals(1L << 30, ByteUnit.GiB.toBytes(1));
+ assertEquals(1L << 40, ByteUnit.TiB.toBytes(1));
+ assertEquals(1L << 50, ByteUnit.PiB.toBytes(1));
+ }
+
+ @Test
+ void toBytesRejectsNegativeValues() {
+ assertThrows(IllegalArgumentException.class, () ->
ByteUnit.KiB.toBytes(-1));
+ assertThrows(IllegalArgumentException.class, () ->
ByteUnit.PiB.toBytes(Long.MIN_VALUE));
+ }
+
+ @Test
+ void toBytesRejectsOverflow() {
+ // PiB has multiplier 2^50, so any value above Long.MAX_VALUE / 2^50 (=
8191) overflows.
+ assertThrows(IllegalArgumentException.class, () ->
ByteUnit.PiB.toBytes(8192));
+ assertThrows(IllegalArgumentException.class, () ->
ByteUnit.PiB.toBytes(Long.MAX_VALUE));
+ // GiB has multiplier 2^30, so Long.MAX_VALUE / 2^30 (= 8589934591) is the
last safe value.
+ assertThrows(IllegalArgumentException.class, () ->
ByteUnit.GiB.toBytes(8589934592L));
+ }
+
+ @Test
+ void toBytesAcceptsLargestNonOverflowingValue() {
+ // Long.MAX_VALUE / 2^50 == 8191 is the largest PiB value that still fits
in a long; the
+ // result is pinned to an independent constant so a wrong product or a
misfiring guard is
+ // caught rather than recomputed on both sides.
+ long bytes = ByteUnit.PiB.toBytes(8191);
+ assertEquals(9222246136947933184L, bytes);
+ assertTrue(bytes > 0, "conversion at the boundary must stay positive");
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
