From 552b8c16da6559112f8cbbb4bab92bc49617ff4a Mon Sep 17 00:00:00 2001 From: pingpingy1 Date: Wed, 14 Feb 2024 15:27:02 +0900 Subject: [PATCH 1/2] Guard Values against null/empty values The classes modified by this commit are `DoubleValue`, `LongValue`, and `TimeValue`. Both `null` and empty strings provided to their constructors fail, but they provide very different error messages (NullPointerException and StringIndexOutOfBoundsException), which is neither sensible nor helpful in debugging. This commit adds a guard to throw `IllegalArgumentException` for both cases in order to improve coherency and usefulness of the error messages. --- .../sf/jsqlparser/expression/DoubleValue.java | 3 ++ .../sf/jsqlparser/expression/LongValue.java | 3 ++ .../sf/jsqlparser/expression/TimeValue.java | 3 ++ .../expression/DoubleValueTest.java | 31 +++++++++++++++++++ .../jsqlparser/expression/LongValueTest.java | 15 +++++++++ .../jsqlparser/expression/TimeValueTest.java | 31 +++++++++++++++++++ 6 files changed, 86 insertions(+) create mode 100644 src/test/java/net/sf/jsqlparser/expression/DoubleValueTest.java create mode 100644 src/test/java/net/sf/jsqlparser/expression/TimeValueTest.java diff --git a/src/main/java/net/sf/jsqlparser/expression/DoubleValue.java b/src/main/java/net/sf/jsqlparser/expression/DoubleValue.java index 43e072dcf..910ad722e 100644 --- a/src/main/java/net/sf/jsqlparser/expression/DoubleValue.java +++ b/src/main/java/net/sf/jsqlparser/expression/DoubleValue.java @@ -24,6 +24,9 @@ public DoubleValue() { } public DoubleValue(final String value) { + if (value == null || value.length() == 0) { + throw new IllegalArgumentException("value can neither be null nor empty."); + } String val = value; if (val.charAt(0) == '+') { val = val.substring(1); diff --git a/src/main/java/net/sf/jsqlparser/expression/LongValue.java b/src/main/java/net/sf/jsqlparser/expression/LongValue.java index 014802fb5..730a8871e 100644 --- a/src/main/java/net/sf/jsqlparser/expression/LongValue.java +++ b/src/main/java/net/sf/jsqlparser/expression/LongValue.java @@ -26,6 +26,9 @@ public LongValue() { } public LongValue(final String value) { + if (value == null || value.length() == 0) { + throw new IllegalArgumentException("value can neither be null nor empty."); + } String val = value; if (val.charAt(0) == '+') { val = val.substring(1); diff --git a/src/main/java/net/sf/jsqlparser/expression/TimeValue.java b/src/main/java/net/sf/jsqlparser/expression/TimeValue.java index 100945147..f87970fa1 100644 --- a/src/main/java/net/sf/jsqlparser/expression/TimeValue.java +++ b/src/main/java/net/sf/jsqlparser/expression/TimeValue.java @@ -25,6 +25,9 @@ public TimeValue() { } public TimeValue(String value) { + if (value == null || value.length() == 0) { + throw new IllegalArgumentException("value can neither be null nor empty."); + } this.value = Time.valueOf(value.substring(1, value.length() - 1)); } diff --git a/src/test/java/net/sf/jsqlparser/expression/DoubleValueTest.java b/src/test/java/net/sf/jsqlparser/expression/DoubleValueTest.java new file mode 100644 index 000000000..1ef676c6d --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/expression/DoubleValueTest.java @@ -0,0 +1,31 @@ +/*- + * #%L + * JSQLParser library + * %% + * Copyright (C) 2004 - 2019 JSQLParser + * %% + * Dual licensed under GNU LGPL 2.1 or Apache License 2.0 + * #L% + */ +package net.sf.jsqlparser.expression; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class DoubleValueTest { + + @Test + public void testNullValue() { + assertThrows(IllegalArgumentException.class, () -> { + new DoubleValue(null); + }); + } + + @Test + public void testEmptyValue() { + assertThrows(IllegalArgumentException.class, () -> { + new DoubleValue(""); + }); + } +} diff --git a/src/test/java/net/sf/jsqlparser/expression/LongValueTest.java b/src/test/java/net/sf/jsqlparser/expression/LongValueTest.java index 91905614f..de1a8ca86 100644 --- a/src/test/java/net/sf/jsqlparser/expression/LongValueTest.java +++ b/src/test/java/net/sf/jsqlparser/expression/LongValueTest.java @@ -11,6 +11,7 @@ import java.math.BigInteger; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import org.junit.jupiter.api.Test; @@ -43,4 +44,18 @@ public void testLargeNumber() { } assertEquals(new BigInteger(largeNumber), value.getBigIntegerValue()); } + + @Test + public void testNullStringValue() { + assertThrows(IllegalArgumentException.class, () -> { + new LongValue((String) null); + }); + } + + @Test + public void testEmptyStringValue() { + assertThrows(IllegalArgumentException.class, () -> { + new LongValue(""); + }); + } } diff --git a/src/test/java/net/sf/jsqlparser/expression/TimeValueTest.java b/src/test/java/net/sf/jsqlparser/expression/TimeValueTest.java new file mode 100644 index 000000000..c931dbcd8 --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/expression/TimeValueTest.java @@ -0,0 +1,31 @@ +/*- + * #%L + * JSQLParser library + * %% + * Copyright (C) 2004 - 2019 JSQLParser + * %% + * Dual licensed under GNU LGPL 2.1 or Apache License 2.0 + * #L% + */ +package net.sf.jsqlparser.expression; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class TimeValueTest { + + @Test + public void testNullValue() { + assertThrows(IllegalArgumentException.class, () -> { + new TimeValue(null); + }); + } + + @Test + public void testEmptyValue() { + assertThrows(IllegalArgumentException.class, () -> { + new TimeValue(""); + }); + } +} From 0bce4648221a242887eef3a213a650dac65dc381 Mon Sep 17 00:00:00 2001 From: pingpingy1 Date: Wed, 14 Feb 2024 16:09:47 +0900 Subject: [PATCH 2/2] fix checkstyle issues --- src/test/java/net/sf/jsqlparser/expression/LongValueTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/net/sf/jsqlparser/expression/LongValueTest.java b/src/test/java/net/sf/jsqlparser/expression/LongValueTest.java index de1a8ca86..ab8119392 100644 --- a/src/test/java/net/sf/jsqlparser/expression/LongValueTest.java +++ b/src/test/java/net/sf/jsqlparser/expression/LongValueTest.java @@ -40,7 +40,7 @@ public void testLargeNumber() { value.getValue(); fail("should not work"); } catch (Exception e) { - //expected to fail + // expected to fail } assertEquals(new BigInteger(largeNumber), value.getBigIntegerValue()); }