Skip to content

Commit 4332a04

Browse files
ronavevacursoragent
andcommitted
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<String,String>, which compiled because of generic type erasure but threw ClassCastException at runtime when the value was e.g. Integer (the case reported in #32 and PagerDuty ticket RUN-2569). Restructure setHeaders to delegate parsing to a private parseHeaders helper that returns Map<String,Object> 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 <cursoragent@cursor.com>
1 parent fb86059 commit 4332a04

2 files changed

Lines changed: 127 additions & 24 deletions

File tree

src/main/java/edu/ohio/ais/rundeck/HttpBuilder.java

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -429,38 +429,46 @@ String getAuthHeader(PluginStepContext pluginStepContext, Map<String, Object> o
429429

430430

431431
public void setHeaders(String headers, RequestBuilder request){
432-
//checking json
433-
Gson gson = new Gson();
434-
Map<String,String> map = new HashMap<>();
432+
Map<String, Object> map = parseHeaders(headers);
433+
if (map == null) {
434+
log.log(0, "Error parsing the headers");
435+
return;
436+
}
437+
for (Map.Entry<String, Object> entry : map.entrySet()) {
438+
request.setHeader(entry.getKey(), headerValueToString(entry.getValue()));
439+
}
440+
}
435441

442+
@SuppressWarnings("unchecked")
443+
private static Map<String, Object> parseHeaders(String headers) {
436444
try {
437-
map = (Map<String,String>) gson.fromJson(headers, map.getClass());
438-
} catch (Exception e) {
439-
map = null;
445+
Object parsed = new Gson().fromJson(headers, HashMap.class);
446+
if (parsed instanceof Map) {
447+
return (Map<String, Object>) parsed;
448+
}
449+
} catch (Exception ignored) {
450+
// fall through to YAML
440451
}
441-
442-
//checking yml
443-
if(map == null) {
444-
map = new HashMap<>();
445-
Object object = null;
446-
try {
447-
Yaml yaml = new Yaml(new SafeConstructor(new LoaderOptions()));
448-
map = yaml.load(headers);
449-
} catch (Exception e) {
450-
map = null;
452+
try {
453+
Object parsed = new Yaml(new SafeConstructor(new LoaderOptions())).load(headers);
454+
if (parsed instanceof Map) {
455+
return (Map<String, Object>) parsed;
451456
}
457+
} catch (Exception ignored) {
458+
// both parsers failed; caller logs and skips
452459
}
460+
return null;
461+
}
453462

454-
if(map == null){
455-
log.log(0, "Error parsing the headers");
456-
}else{
457-
for (Map.Entry<String, String> entry : map.entrySet()) {
458-
String key = entry.getKey();
459-
String value = entry.getValue();
460-
461-
request.setHeader(key, value);
463+
static String headerValueToString(Object value) {
464+
if (value instanceof Double) {
465+
double d = (Double) value;
466+
if (!Double.isInfinite(d) && !Double.isNaN(d) && d == Math.floor(d)
467+
&& d >= Long.MIN_VALUE && d <= Long.MAX_VALUE) {
468+
return Long.toString((long) d);
462469
}
463470
}
471+
return String.valueOf(value);
464472
}
465473

466474
static void propertyResolver(String pluginType, String property, Map<String,Object> Configuration, PluginStepContext context, String SERVICE_PROVIDER_NAME) {

src/test/java/edu/ohio/ais/rundeck/HttpBuilderTest.java

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package edu.ohio.ais.rundeck;
22

3+
import com.dtolabs.rundeck.plugins.PluginLogger;
4+
import org.apache.http.client.methods.RequestBuilder;
5+
import org.junit.Before;
36
import org.junit.Test;
47

58
import java.util.HashMap;
@@ -8,9 +11,21 @@
811
import static edu.ohio.ais.rundeck.HttpBuilder.*;
912
import static org.junit.Assert.assertEquals;
1013
import static org.junit.Assert.assertNull;
14+
import static org.mockito.Mockito.mock;
15+
import static org.mockito.Mockito.verify;
16+
import static org.mockito.Mockito.verifyZeroInteractions;
1117

1218
public class HttpBuilderTest {
1319

20+
private HttpBuilder builder;
21+
private RequestBuilder request;
22+
23+
@Before
24+
public void setUp() {
25+
builder = new HttpBuilder();
26+
request = mock(RequestBuilder.class);
27+
}
28+
1429
@Test
1530
public void testGetStringOption() {
1631
Map<String, Object> options = new HashMap<>();
@@ -75,4 +90,84 @@ public void testGetBooleanOption() {
7590
assertEquals("Expected the value associated with the key", Boolean.FALSE, result);
7691
}
7792

93+
// RUN-2569 / upstream issue #32: HttpBuilder.setHeaders previously threw
94+
// ClassCastException when YAML or JSON header values parsed as non-String
95+
// scalars (e.g. `Content-Length: 0`). These tests pin the fixed behavior.
96+
97+
@Test
98+
public void setHeaders_yamlIntegerValue_setsStringifiedValue() {
99+
builder.setHeaders("Content-Length: 0", request);
100+
verify(request).setHeader("Content-Length", "0");
101+
}
102+
103+
@Test
104+
public void setHeaders_jsonIntegerValue_setsStringifiedValue() {
105+
builder.setHeaders("{\"Content-Length\":0}", request);
106+
verify(request).setHeader("Content-Length", "0");
107+
}
108+
109+
@Test
110+
public void setHeaders_yamlStringValue_setsValueUnchanged() {
111+
builder.setHeaders("Authorization: Bearer abc", request);
112+
verify(request).setHeader("Authorization", "Bearer abc");
113+
}
114+
115+
@Test
116+
public void setHeaders_jsonStringValue_setsValueUnchanged() {
117+
builder.setHeaders("{\"X-Custom\":\"foo\"}", request);
118+
verify(request).setHeader("X-Custom", "foo");
119+
}
120+
121+
@Test
122+
public void setHeaders_yamlMixedStringAndNumeric_setsBothCorrectly() {
123+
builder.setHeaders("Content-Length: 0\nX-Custom: foo", request);
124+
verify(request).setHeader("Content-Length", "0");
125+
verify(request).setHeader("X-Custom", "foo");
126+
}
127+
128+
@Test
129+
public void setHeaders_yamlBooleanValue_setsStringifiedValue() {
130+
builder.setHeaders("X-Debug: true", request);
131+
verify(request).setHeader("X-Debug", "true");
132+
}
133+
134+
@Test
135+
public void setHeaders_yamlListValue_setsStringifiedValueWithoutThrowing() {
136+
// Non-scalar values no longer throw ClassCastException; they are
137+
// coerced to their default toString() representation. This is a
138+
// deliberate behavior change relative to the pre-fix code, which
139+
// crashed on any non-String value.
140+
builder.setHeaders("Accept: [a, b]", request);
141+
verify(request).setHeader("Accept", "[a, b]");
142+
}
143+
144+
@Test
145+
public void setHeaders_unparseableInput_logsAndDoesNotThrow() {
146+
PluginLogger log = mock(PluginLogger.class);
147+
builder.setLog(log);
148+
// A bareword is neither valid JSON nor a YAML map; both parsers
149+
// either throw or return a non-Map value, so setHeaders must log
150+
// the parse error and skip touching the request.
151+
builder.setHeaders("not a map", request);
152+
verify(log).log(0, "Error parsing the headers");
153+
verifyZeroInteractions(request);
154+
}
155+
156+
@Test
157+
public void headerValueToString_wholeNumberDouble_emitsIntegerString() {
158+
// Gson parses all JSON numbers as Double by default; ensure that a
159+
// whole-number Double like 0.0 is emitted as "0" rather than "0.0"
160+
// so headers like Content-Length stay valid for strict HTTP servers.
161+
assertEquals("0", headerValueToString(0.0));
162+
assertEquals("1024", headerValueToString(1024.0));
163+
assertEquals("-1", headerValueToString(-1.0));
164+
}
165+
166+
@Test
167+
public void headerValueToString_fractionalDouble_emitsDecimalString() {
168+
// Genuinely fractional Doubles must keep their decimal form rather
169+
// than being silently truncated to a long.
170+
assertEquals("1.5", headerValueToString(1.5));
171+
}
172+
78173
}

0 commit comments

Comments
 (0)