Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ public void copyFrom(IVariables variables) {
// the same object as the argument.
String[] variableNames = variables.getVariableNames();
for (String variableName : variableNames) {
if (Utils.isEmpty(variableName)) {
continue;
}
properties.put(variableName, variables.getVariable(variableName));
}
}
Expand Down Expand Up @@ -140,6 +143,11 @@ public String[] getVariableNames() {

@Override
public synchronized void setVariable(String variableName, String variableValue) {
// Reject null/empty names: HashMap allows null keys, but callers that iterate
// names and check Immutable Sets (Set.of) throw NPE on contains(null) — issue #7067.
if (Utils.isEmpty(variableName)) {
return;
}
if (variableValue != null) {
properties.put(variableName, variableValue);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,23 @@ void testEnvironmentSubstitute() {
new String[] {"DataOne", "TheDataOne"},
vars.resolve(new String[] {"${VarOne}", "The${VarOne}"}));
}

/**
* Null or empty variable names must not enter the properties map (issue #7067). A null key caused
* NPE later when checking Const.INTERNAL_*_VARIABLES Set.of collections.
*/
@Test
void setVariableIgnoresNullAndEmptyNames() {
Variables vars = new Variables();
vars.setVariable(null, "shouldNotStore");
vars.setVariable("", "shouldNotStore");
vars.setVariable("valid", "ok");

for (String name : vars.getVariableNames()) {
assertTrue(name != null && !name.isEmpty());
}
assertEquals("ok", vars.getVariable("valid"));
assertNull(vars.getVariable(null));
assertNull(vars.getVariable(""));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,10 @@ public static void addMissingVariables(IVariables fromSpace, IVariables toSpace)
}
String[] variableNames = toSpace.getVariableNames();
for (String variable : variableNames) {
// Skip null/empty names: Set.contains(null) throws NPE and empty keys are invalid
if (Utils.isEmpty(variable)) {
continue;
}
if (fromSpace.getVariable(variable) == null) {
fromSpace.setVariable(variable, toSpace.getVariable(variable));
}
Expand All @@ -326,6 +330,10 @@ public static void replaceVariableValues(
}
String[] variableNames = replaceBy.getVariableNames();
for (String variableName : variableNames) {
// Skip null/empty names: Immutable Set.contains(null) throws NPE (issue #7067)
if (Utils.isEmpty(variableName)) {
continue;
}
if (childPipelineMeta.getVariable(variableName) != null
&& !isInternalVariable(variableName, type)) {
childPipelineMeta.setVariable(variableName, replaceBy.getVariable(variableName));
Expand All @@ -347,10 +355,10 @@ private static boolean isInternalVariable(String variableName, String type) {
}

private static boolean isPipelineInternalVariable(String variableName) {
return Const.INTERNAL_PIPELINE_VARIABLES.contains(variableName);
return variableName != null && Const.INTERNAL_PIPELINE_VARIABLES.contains(variableName);
}

private static boolean isWorkflowInternalVariable(String variableName) {
return Const.INTERNAL_WORKFLOW_VARIABLES.contains(variableName);
return variableName != null && Const.INTERNAL_WORKFLOW_VARIABLES.contains(variableName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@
*/
package org.apache.hop.pipeline;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.apache.hop.core.Const;
import org.apache.hop.core.HopEnvironment;
Expand Down Expand Up @@ -168,4 +172,39 @@ void replaceVariablesWithPipelineInternalVariablesTest() {
// keep child only variables
assertEquals(variableChildOnly, childVariables.getVariable(variableChildOnly));
}

/**
* Reproduces issue #7067: a null variable name in the parent space used to NPE when checking
* internal variable sets (Set.of(...).contains(null)). Async web services could inject a null key
* when header content variable was unset.
*/
@Test
void replaceVariableValuesSkipsNullVariableNames() {
String variableOverwrite = "paramOverwrite";
IVariables childVariables = new Variables();
childVariables.setVariable(variableOverwrite, "childValue");

IVariables replaceByParentVariables = mock(IVariables.class);
when(replaceByParentVariables.getVariableNames())
.thenReturn(new String[] {null, "", variableOverwrite});
when(replaceByParentVariables.getVariable(variableOverwrite)).thenReturn("parentValue");

assertDoesNotThrow(
() ->
TransformWithMappingMeta.replaceVariableValues(
childVariables, replaceByParentVariables));
assertEquals("parentValue", childVariables.getVariable(variableOverwrite));
}

@Test
void addMissingVariablesSkipsNullVariableNames() {
IVariables fromSpace = new Variables();
IVariables toSpace = mock(IVariables.class);
when(toSpace.getVariableNames()).thenReturn(new String[] {null, "", "addedVar"});
when(toSpace.getVariable("addedVar")).thenReturn("addedValue");

assertDoesNotThrow(() -> TransformWithMappingMeta.addMissingVariables(fromSpace, toSpace));
assertEquals("addedValue", fromSpace.getVariable("addedVar"));
assertNull(fromSpace.getVariable(null));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,13 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) {
workflow.initializeFrom(variables);
workflow.setVariable("SERVER_OBJECT_ID", serverObjectId);

// See if we need to pass a variable with the content in it...
//
// Read the content posted?
// Pass body and header content into variables when configured.
// Guard both independently so an unset header variable never injects a null key
// (which caused NPE in Kafka Consumer init via replaceVariableValues — issue #7067).
//
String contentVariable = variables.resolve(webService.getBodyContentVariable());
String headerContentVariable = variables.resolve(webService.getHeaderContentVariable());

String content = "";
if (StringUtils.isNotEmpty(contentVariable)) {
try (InputStream in = request.getInputStream()) {
Expand All @@ -193,24 +195,21 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) {
}
}
workflow.setVariable(contentVariable, Const.NVL(content, ""));
}

String headerContentVariable = variables.resolve(webService.getHeaderContentVariable());
String headerContent = "";
if (StringUtils.isNotEmpty(headerContentVariable)) {
// Create JSON object containing all request headers
ObjectMapper objectMapper = new ObjectMapper();
ObjectNode headersJson = objectMapper.createObjectNode();

Enumeration<String> headerNames = request.getHeaderNames();
while (headerNames.hasMoreElements()) {
String headerName = headerNames.nextElement();
String headerValue = request.getHeader(headerName);
headersJson.put(headerName, headerValue);
}
headerContent = objectMapper.writeValueAsString(headersJson);
}
if (StringUtils.isNotEmpty(headerContentVariable)) {
// Create JSON object containing all request headers
ObjectMapper objectMapper = new ObjectMapper();
ObjectNode headersJson = objectMapper.createObjectNode();

workflow.setVariable(headerContentVariable, headerContent);
Enumeration<String> headerNames = request.getHeaderNames();
while (headerNames.hasMoreElements()) {
String headerName = headerNames.nextElement();
String headerValue = request.getHeader(headerName);
headersJson.put(headerName, headerValue);
}
String headerContent = objectMapper.writeValueAsString(headersJson);
workflow.setVariable(headerContentVariable, Const.NVL(headerContent, ""));
}

// Set all the other parameters as variables/parameters...
Expand Down
Loading