From 4332a049a9bc2fed16475cf407cf07f1fbeaf7aa Mon Sep 17 00:00:00 2001 From: Rodrigo Navarro Date: Tue, 19 May 2026 11:49:33 -0400 Subject: [PATCH] fix: coerce non-String header values in HttpBuilder.setHeaders (RUN-2569) HttpBuilder.setHeaders parsed the headers config string with Gson, then fell back to SnakeYAML. Both libraries place non-String values into the resulting map for JSON/YAML numeric and boolean scalars. The map was then iterated with Map.Entry, which compiled because of generic type erasure but threw ClassCastException at runtime when the value was e.g. Integer (the case reported in https://github.com/rundeck-plugins/http-step/issues/32 and PagerDuty ticket RUN-2569). Restructure setHeaders to delegate parsing to a private parseHeaders helper that returns Map after explicit `instanceof Map` checks (no more "exceptions as control flow"), and route every value through a new headerValueToString helper. The helper renders whole-number Double values via Long.toString so JSON like {"Content-Length":0} emits "0" instead of "0.0" (Gson parses every JSON number as Double when the target type is HashMap.class). Non-scalar values (lists, nested maps) are stringified via String.valueOf instead of throwing. Adds unit tests in HttpBuilderTest covering: YAML/JSON integer, plain string, mixed types, boolean, list stringification, unparseable input log path, and direct tests of headerValueToString for whole-number and fractional Doubles. ./gradlew clean build green on Java 17; HttpBuilderTest 14/14. Co-authored-by: Cursor --- .../edu/ohio/ais/rundeck/HttpBuilder.java | 56 ++++++----- .../edu/ohio/ais/rundeck/HttpBuilderTest.java | 95 +++++++++++++++++++ 2 files changed, 127 insertions(+), 24 deletions(-) diff --git a/src/main/java/edu/ohio/ais/rundeck/HttpBuilder.java b/src/main/java/edu/ohio/ais/rundeck/HttpBuilder.java index 3150d23..98ff28b 100644 --- a/src/main/java/edu/ohio/ais/rundeck/HttpBuilder.java +++ b/src/main/java/edu/ohio/ais/rundeck/HttpBuilder.java @@ -429,38 +429,46 @@ String getAuthHeader(PluginStepContext pluginStepContext, Map o public void setHeaders(String headers, RequestBuilder request){ - //checking json - Gson gson = new Gson(); - Map map = new HashMap<>(); + Map map = parseHeaders(headers); + if (map == null) { + log.log(0, "Error parsing the headers"); + return; + } + for (Map.Entry entry : map.entrySet()) { + request.setHeader(entry.getKey(), headerValueToString(entry.getValue())); + } + } + @SuppressWarnings("unchecked") + private static Map parseHeaders(String headers) { try { - map = (Map) gson.fromJson(headers, map.getClass()); - } catch (Exception e) { - map = null; + Object parsed = new Gson().fromJson(headers, HashMap.class); + if (parsed instanceof Map) { + return (Map) parsed; + } + } catch (Exception ignored) { + // fall through to YAML } - - //checking yml - if(map == null) { - map = new HashMap<>(); - Object object = null; - try { - Yaml yaml = new Yaml(new SafeConstructor(new LoaderOptions())); - map = yaml.load(headers); - } catch (Exception e) { - map = null; + try { + Object parsed = new Yaml(new SafeConstructor(new LoaderOptions())).load(headers); + if (parsed instanceof Map) { + return (Map) parsed; } + } catch (Exception ignored) { + // both parsers failed; caller logs and skips } + return null; + } - if(map == null){ - log.log(0, "Error parsing the headers"); - }else{ - for (Map.Entry entry : map.entrySet()) { - String key = entry.getKey(); - String value = entry.getValue(); - - request.setHeader(key, value); + static String headerValueToString(Object value) { + if (value instanceof Double) { + double d = (Double) value; + if (!Double.isInfinite(d) && !Double.isNaN(d) && d == Math.floor(d) + && d >= Long.MIN_VALUE && d <= Long.MAX_VALUE) { + return Long.toString((long) d); } } + return String.valueOf(value); } static void propertyResolver(String pluginType, String property, Map Configuration, PluginStepContext context, String SERVICE_PROVIDER_NAME) { diff --git a/src/test/java/edu/ohio/ais/rundeck/HttpBuilderTest.java b/src/test/java/edu/ohio/ais/rundeck/HttpBuilderTest.java index 91189a6..8fa0c4f 100644 --- a/src/test/java/edu/ohio/ais/rundeck/HttpBuilderTest.java +++ b/src/test/java/edu/ohio/ais/rundeck/HttpBuilderTest.java @@ -1,5 +1,8 @@ package edu.ohio.ais.rundeck; +import com.dtolabs.rundeck.plugins.PluginLogger; +import org.apache.http.client.methods.RequestBuilder; +import org.junit.Before; import org.junit.Test; import java.util.HashMap; @@ -8,9 +11,21 @@ import static edu.ohio.ais.rundeck.HttpBuilder.*; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; public class HttpBuilderTest { + private HttpBuilder builder; + private RequestBuilder request; + + @Before + public void setUp() { + builder = new HttpBuilder(); + request = mock(RequestBuilder.class); + } + @Test public void testGetStringOption() { Map options = new HashMap<>(); @@ -75,4 +90,84 @@ public void testGetBooleanOption() { assertEquals("Expected the value associated with the key", Boolean.FALSE, result); } + // RUN-2569 / upstream issue #32: HttpBuilder.setHeaders previously threw + // ClassCastException when YAML or JSON header values parsed as non-String + // scalars (e.g. `Content-Length: 0`). These tests pin the fixed behavior. + + @Test + public void setHeaders_yamlIntegerValue_setsStringifiedValue() { + builder.setHeaders("Content-Length: 0", request); + verify(request).setHeader("Content-Length", "0"); + } + + @Test + public void setHeaders_jsonIntegerValue_setsStringifiedValue() { + builder.setHeaders("{\"Content-Length\":0}", request); + verify(request).setHeader("Content-Length", "0"); + } + + @Test + public void setHeaders_yamlStringValue_setsValueUnchanged() { + builder.setHeaders("Authorization: Bearer abc", request); + verify(request).setHeader("Authorization", "Bearer abc"); + } + + @Test + public void setHeaders_jsonStringValue_setsValueUnchanged() { + builder.setHeaders("{\"X-Custom\":\"foo\"}", request); + verify(request).setHeader("X-Custom", "foo"); + } + + @Test + public void setHeaders_yamlMixedStringAndNumeric_setsBothCorrectly() { + builder.setHeaders("Content-Length: 0\nX-Custom: foo", request); + verify(request).setHeader("Content-Length", "0"); + verify(request).setHeader("X-Custom", "foo"); + } + + @Test + public void setHeaders_yamlBooleanValue_setsStringifiedValue() { + builder.setHeaders("X-Debug: true", request); + verify(request).setHeader("X-Debug", "true"); + } + + @Test + public void setHeaders_yamlListValue_setsStringifiedValueWithoutThrowing() { + // Non-scalar values no longer throw ClassCastException; they are + // coerced to their default toString() representation. This is a + // deliberate behavior change relative to the pre-fix code, which + // crashed on any non-String value. + builder.setHeaders("Accept: [a, b]", request); + verify(request).setHeader("Accept", "[a, b]"); + } + + @Test + public void setHeaders_unparseableInput_logsAndDoesNotThrow() { + PluginLogger log = mock(PluginLogger.class); + builder.setLog(log); + // A bareword is neither valid JSON nor a YAML map; both parsers + // either throw or return a non-Map value, so setHeaders must log + // the parse error and skip touching the request. + builder.setHeaders("not a map", request); + verify(log).log(0, "Error parsing the headers"); + verifyZeroInteractions(request); + } + + @Test + public void headerValueToString_wholeNumberDouble_emitsIntegerString() { + // Gson parses all JSON numbers as Double by default; ensure that a + // whole-number Double like 0.0 is emitted as "0" rather than "0.0" + // so headers like Content-Length stay valid for strict HTTP servers. + assertEquals("0", headerValueToString(0.0)); + assertEquals("1024", headerValueToString(1024.0)); + assertEquals("-1", headerValueToString(-1.0)); + } + + @Test + public void headerValueToString_fractionalDouble_emitsDecimalString() { + // Genuinely fractional Doubles must keep their decimal form rather + // than being silently truncated to a long. + assertEquals("1.5", headerValueToString(1.5)); + } + }