From 3b7cf55178ddc4fa5fcad5e29b2b2ac21ded8bbb Mon Sep 17 00:00:00 2001 From: Eunbin Son Date: Sat, 20 Jun 2026 09:41:15 +0900 Subject: [PATCH 1/2] Dell: Fix EcsURI null-location check that never triggers The EcsURI(String location) constructor passed the boolean expression `location == null` as the first argument to Preconditions.checkNotNull. checkNotNull's first argument is the reference to null-check, so the autoboxed Boolean was always non-null and the guard never fired. When location was null, the check passed and URI.create(location) threw a NullPointerException with no message. Pass `location` itself as the reference so the intended null check runs and produces the "Location ... can not be null" message. Generated-by: Claude Code --- dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java | 2 +- .../test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java b/dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java index f5d2022e0bda..3eac5d7273f1 100644 --- a/dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java +++ b/dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java @@ -35,7 +35,7 @@ class EcsURI { private final String name; EcsURI(String location) { - Preconditions.checkNotNull(location == null, "Location %s can not be null", location); + Preconditions.checkNotNull(location, "Location %s can not be null", location); this.location = location; diff --git a/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java b/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java index ec56e76bb717..c6863b458dbb 100644 --- a/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java +++ b/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java @@ -59,4 +59,11 @@ public void testInvalidLocation() { .isInstanceOf(ValidationException.class) .hasMessage("Invalid ecs location: http://bucket/a"); } + + @Test + public void testNullLocation() { + assertThatThrownBy(() -> new EcsURI((String) null)) + .isInstanceOf(NullPointerException.class) + .hasMessageContaining("can not be null"); + } } From 56e1ac728b43570d6fd954b1bf9bdd8505b44e6f Mon Sep 17 00:00:00 2001 From: Eunbin Son Date: Mon, 22 Jun 2026 07:23:03 +0900 Subject: [PATCH 2/2] Dell: Address review comments on EcsURI null-check - Drop redundant "null" from the message ("Location null can not be null" -> "Location can not be null"). - Remove unnecessary (String) cast in TestEcsURI; the single-arg constructor is unambiguous. - Use hasMessage() instead of hasMessageContaining() to match testInvalidLocation. Co-Authored-By: Claude Opus 4.8 (1M context) --- dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java | 2 +- .../src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java b/dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java index 3eac5d7273f1..3f895972f732 100644 --- a/dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java +++ b/dell/src/main/java/org/apache/iceberg/dell/ecs/EcsURI.java @@ -35,7 +35,7 @@ class EcsURI { private final String name; EcsURI(String location) { - Preconditions.checkNotNull(location, "Location %s can not be null", location); + Preconditions.checkNotNull(location, "Location can not be null"); this.location = location; diff --git a/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java b/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java index c6863b458dbb..6566dc25f92f 100644 --- a/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java +++ b/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsURI.java @@ -62,8 +62,8 @@ public void testInvalidLocation() { @Test public void testNullLocation() { - assertThatThrownBy(() -> new EcsURI((String) null)) + assertThatThrownBy(() -> new EcsURI(null)) .isInstanceOf(NullPointerException.class) - .hasMessageContaining("can not be null"); + .hasMessage("Location can not be null"); } }