From b5c10c2301a5f58805c3a670261b07321fd0ac7d Mon Sep 17 00:00:00 2001 From: mugeon Date: Sat, 25 Oct 2025 02:09:45 +0900 Subject: [PATCH 1/2] Fix JobParameter validation bug in compact constructor - Fixed line 41 to validate 'name' parameter instead of 'value' - The 'name' parameter was previously not validated, allowing null values - This caused potential NullPointerException in equals() and hashCode() methods - Added test case 'testAddingNullJobParameters' to verify the fix Fixes gh-5049 Signed-off-by: mugeon --- .../batch/core/job/parameters/JobParameter.java | 2 +- .../batch/core/JobParameterTests.java | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/job/parameters/JobParameter.java b/spring-batch-core/src/main/java/org/springframework/batch/core/job/parameters/JobParameter.java index 30417e60f0..ab40f81c17 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/job/parameters/JobParameter.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/job/parameters/JobParameter.java @@ -47,7 +47,7 @@ public record JobParameter(String name, T value, Class type, boolean ident * @since 6.0 */ public JobParameter { - Assert.notNull(value, "name must not be null"); + Assert.notNull(name, "name must not be null"); Assert.notNull(value, "value must not be null"); Assert.notNull(type, "type must not be null"); } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java index ce4d22b220..b919500ade 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java @@ -15,14 +15,15 @@ */ package org.springframework.batch.core; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.util.Date; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.batch.core.job.parameters.JobParameter; +import org.springframework.batch.core.job.parameters.JobParametersBuilder; + +import static org.junit.jupiter.api.Assertions.*; /** * @author Lucas Ward @@ -84,4 +85,11 @@ void testHashcode() { assertEquals(testParameter.hashCode(), jobParameter.hashCode()); } + @Test + void testAddingNullJobParameters() { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> new JobParametersBuilder().addString("foo", null).toJobParameters()); + Assertions.assertEquals("Value for parameter 'foo' must not be null", exception.getMessage()); + } + } \ No newline at end of file From 839089500e078e2a8c1a346576534051d554b4bc Mon Sep 17 00:00:00 2001 From: mugeon Date: Sat, 25 Oct 2025 03:38:51 +0900 Subject: [PATCH 2/2] Improve JobParameter validation tests - Removed testAddingNullJobParameters (indirect test via JobParametersBuilder) - Added testConstructorNameNotNull to directly test name validation - Added testConstructorValueNotNull to directly test value validation - Added testConstructorTypeNotNull to directly test type validation - These tests directly validate the constructor fix Signed-off-by: mugeon --- .../batch/core/JobParameterTests.java | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java index b919500ade..f8e1bd59e4 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java @@ -17,11 +17,9 @@ import java.util.Date; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.batch.core.job.parameters.JobParameter; -import org.springframework.batch.core.job.parameters.JobParametersBuilder; import static org.junit.jupiter.api.Assertions.*; @@ -32,6 +30,27 @@ */ class JobParameterTests { + @Test + void testConstructorNameNotNull() { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> new JobParameter<>(null, "test", String.class, true)); + assertEquals("name must not be null", exception.getMessage()); + } + + @Test + void testConstructorValueNotNull() { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> new JobParameter<>("param", null, String.class, true)); + assertEquals("value must not be null", exception.getMessage()); + } + + @Test + void testConstructorTypeNotNull() { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> new JobParameter<>("param", "test", null, true)); + assertEquals("type must not be null", exception.getMessage()); + } + @Test void testStringParameter() { JobParameter jobParameter = new JobParameter<>("param", "test", String.class, true); @@ -85,11 +104,4 @@ void testHashcode() { assertEquals(testParameter.hashCode(), jobParameter.hashCode()); } - @Test - void testAddingNullJobParameters() { - IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> new JobParametersBuilder().addString("foo", null).toJobParameters()); - Assertions.assertEquals("Value for parameter 'foo' must not be null", exception.getMessage()); - } - } \ No newline at end of file