From 8cd96f995aed5784b4e1d9f60c52a2555bdf3a0a Mon Sep 17 00:00:00 2001 From: Salman Shaikh Date: Mon, 7 Sep 2026 14:54:44 +0300 Subject: [PATCH 1/2] EnvStep: defensively handle null/invalid overrides to avoid NPE and improve error messages --- .../plugins/workflow/steps/EnvStep.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java index da6de349..0c04965d 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java @@ -49,12 +49,16 @@ public class EnvStep extends Step { @DataBoundConstructor public EnvStep(List overrides) { - for (String pair : overrides) { - if (pair.indexOf('=') == -1) { - throw new IllegalArgumentException(pair); + if (overrides == null) { + this.overrides = Collections.emptyList(); + } else { + for (String pair : overrides) { + if (pair == null || pair.indexOf('=') == -1) { + throw new IllegalArgumentException(String.valueOf(pair)); + } } + this.overrides = new ArrayList<>(overrides); } - this.overrides = new ArrayList<>(overrides); } public List getOverrides() { @@ -75,7 +79,7 @@ public static class Execution extends AbstractStepExecutionImpl { Execution(List overrides, StepContext context) { super(context); - this.overrides = overrides; + this.overrides = overrides == null ? Collections.emptyList() : overrides; } @Override @@ -83,8 +87,12 @@ public boolean start() throws Exception { Map overridesM = new HashMap<>(); for (String pair : overrides) { int split = pair.indexOf('='); - assert split != -1; - overridesM.put(pair.substring(0, split), pair.substring(split + 1)); + if (split == -1) { + throw new IllegalStateException("Invalid environment override: " + pair); + } + String key = pair.substring(0, split).trim(); + String value = pair.substring(split + 1); + overridesM.put(key, value); } getContext() .newBodyInvoker() From e7bda5edcc6252e3d0a169acc488afec30426f37 Mon Sep 17 00:00:00 2001 From: Salman Shaikh Date: Mon, 7 Sep 2026 15:14:16 +0300 Subject: [PATCH 2/2] EnvStep: trim values, normalize stored overrides, add unit tests --- .../plugins/workflow/steps/EnvStep.java | 11 +++++-- .../workflow/steps/EnvStepUnitTest.java | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepUnitTest.java diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java index 0c04965d..f8c55cc7 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java @@ -52,12 +52,15 @@ public EnvStep(List overrides) { if (overrides == null) { this.overrides = Collections.emptyList(); } else { + List stored = new ArrayList<>(); for (String pair : overrides) { if (pair == null || pair.indexOf('=') == -1) { throw new IllegalArgumentException(String.valueOf(pair)); } + // store a trimmed form to normalize whitespace (keys/values are trimmed later too) + stored.add(pair.trim()); } - this.overrides = new ArrayList<>(overrides); + this.overrides = stored; } } @@ -88,10 +91,12 @@ public boolean start() throws Exception { for (String pair : overrides) { int split = pair.indexOf('='); if (split == -1) { + // defensive: in case an instance is deserialized from an older form throw new IllegalStateException("Invalid environment override: " + pair); } + // trim both key and value to be tolerant of user input like "FOO = bar " String key = pair.substring(0, split).trim(); - String value = pair.substring(split + 1); + String value = pair.substring(split + 1).trim(); overridesM.put(key, value); } getContext() @@ -143,7 +148,7 @@ public boolean takesImplicitBlockArgument() { public Step newInstance(StaplerRequest2 req, JSONObject formData) throws FormException { String overridesS = formData.getString("overrides"); List overrides = new ArrayList<>(); - for (String line : overridesS.split("\r?\n")) { + for (String line : overridesS.split("\\r?\\n")) { line = line.trim(); if (!line.isEmpty()) { overrides.add(line); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepUnitTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepUnitTest.java new file mode 100644 index 00000000..97c1e0c4 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepUnitTest.java @@ -0,0 +1,29 @@ +package org.jenkinsci.plugins.workflow.steps; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Arrays; +import org.junit.jupiter.api.Test; + +public class EnvStepUnitTest { + + @Test + void constructorAcceptsNull() { + EnvStep s = new EnvStep(null); + assertTrue(s.getOverrides().isEmpty()); + } + + @Test + void constructorRejectsMalformedEntry() { + assertThrows(IllegalArgumentException.class, () -> new EnvStep(Arrays.asList("BADPAIR"))); + } + + @Test + void constructorTrimsStoredPair() { + EnvStep s = new EnvStep(Arrays.asList("FOO = bar ")); + assertEquals(1, s.getOverrides().size()); + assertEquals("FOO = bar", s.getOverrides().get(0)); + } +}