From 2b54b6d7fb9b0f4ad7047c8853074e608dc2acf2 Mon Sep 17 00:00:00 2001 From: Nico Piel Date: Thu, 29 Oct 2026 19:30:54 +0100 Subject: [PATCH 1/3] Add characterization tests for HTTPUtil.parseHeaders Pin the behaviour of the current commons-httpclient 3 based implementation (folding, trimming, duplicates, termination, errors) before replacing it. Signed-off-by: Nico Piel --- .../connect/server/userutil/HTTPUtilTest.java | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 server/src/test/java/com/mirth/connect/server/userutil/HTTPUtilTest.java diff --git a/server/src/test/java/com/mirth/connect/server/userutil/HTTPUtilTest.java b/server/src/test/java/com/mirth/connect/server/userutil/HTTPUtilTest.java new file mode 100644 index 000000000..c1d8936de --- /dev/null +++ b/server/src/test/java/com/mirth/connect/server/userutil/HTTPUtilTest.java @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2026 Nico Piel + +package com.mirth.connect.server.userutil; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; + +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; + +import org.junit.Test; + +/** + * Characterization tests for {@link HTTPUtil#parseHeaders(String)}, pinning the behaviour of the + * original commons-httpclient 3 HttpParser based implementation. + */ +public class HTTPUtilTest { + + private static Map map(String... keyValues) { + Map map = new HashMap<>(); + for (int i = 0; i < keyValues.length; i += 2) { + map.put(keyValues[i], keyValues[i + 1]); + } + return map; + } + + @Test + public void testEmptyString() throws Exception { + assertEquals(map(), HTTPUtil.parseHeaders("")); + } + + @Test + public void testCrlfLines() throws Exception { + assertEquals(map("Content-Type", "text/plain", "Content-Length", "42"), HTTPUtil.parseHeaders("Content-Type: text/plain\r\nContent-Length: 42\r\n")); + } + + @Test + public void testLfLinesAndNoTrailingNewline() throws Exception { + assertEquals(map("A", "1", "B", "2"), HTTPUtil.parseHeaders("A: 1\nB: 2")); + } + + @Test + public void testNameAndValueAreTrimmed() throws Exception { + assertEquals(map("Name", "some value"), HTTPUtil.parseHeaders("Name \t: \t some value \t \r\n")); + assertEquals(map("X", "a"), HTTPUtil.parseHeaders("X:\u000ba\u0001\r\n")); + } + + @Test + public void testValueKeepsFurtherColons() throws Exception { + assertEquals(map("Host", "example.com:8080"), HTTPUtil.parseHeaders("Host: example.com:8080\r\n")); + } + + @Test + public void testEmptyNameAndEmptyValue() throws Exception { + assertEquals(map("", "v", "X", ""), HTTPUtil.parseHeaders(":v\r\nX:\r\n")); + } + + @Test + public void testFoldedContinuationLines() throws Exception { + assertEquals(map("X", "a b c", "Y", "d"), HTTPUtil.parseHeaders("X: a\r\n b \r\n\tc\r\nY: d\r\n")); + } + + @Test + public void testContinuationOfEmptyValue() throws Exception { + assertEquals(map("X", " b"), HTTPUtil.parseHeaders("X:\r\n b\r\n")); + } + + @Test + public void testLeadingContinuationLineIsIgnored() throws Exception { + assertEquals(map("X", "a"), HTTPUtil.parseHeaders(" orphan\r\nX: a\r\n")); + } + + @Test + public void testDuplicateNamesLastWins() throws Exception { + assertEquals(map("X", "second"), HTTPUtil.parseHeaders("X: first\r\nX: second\r\n")); + } + + @Test + public void testNamesAreCaseSensitive() throws Exception { + assertEquals(map("Foo", "1", "foo", "2"), HTTPUtil.parseHeaders("Foo: 1\r\nfoo: 2\r\n")); + } + + @Test + public void testStopsAtBlankLine() throws Exception { + assertEquals(map("A", "1"), HTTPUtil.parseHeaders("A: 1\r\n\r\nB: 2\r\nnot a header\r\n")); + assertEquals(map("A", "1"), HTTPUtil.parseHeaders("A: 1\n\nB: 2\n")); + } + + @Test + public void testStopsAtWhitespaceOnlyLine() throws Exception { + assertEquals(map("A", "1"), HTTPUtil.parseHeaders("A: 1\r\n \t \r\nB: 2\r\n")); + assertEquals(map("A", "1"), HTTPUtil.parseHeaders("A: 1\r\n\r\r\nB: 2\r\n")); + } + + @Test + public void testLeadingBlankLineYieldsNothing() throws Exception { + assertEquals(map(), HTTPUtil.parseHeaders("\r\nA: 1\r\n")); + } + + @Test + public void testCarriageReturnHandling() throws Exception { + // Only a single CR directly before the LF is stripped; others are trimmed or kept + assertEquals(map("A", "x\ry", "B", "w", "C", "z"), HTTPUtil.parseHeaders("A: x\ry\r\nB: w\r\r\nC: z\r")); + } + + @Test + public void testHttpStatusLineIsRejected() { + assertParseFailure("HTTP/1.1 200 OK\r\nA: 1\r\n", "HTTP/1.1 200 OK"); + } + + @Test + public void testLineWithoutColonIsRejected() { + assertParseFailure("A: 1\r\nbogus line\r\n", "bogus line"); + } + + @Test + public void testNonAsciiValue() throws Exception { + // The string is encoded with the platform charset and decoded as UTF-8 + assumeTrue(Charset.defaultCharset().equals(StandardCharsets.UTF_8)); + assertEquals(map("X-Name", "M\u00fcller \u20ac"), HTTPUtil.parseHeaders("X-Name: M\u00fcller \u20ac\r\n")); + } + + @Test(expected = NullPointerException.class) + public void testNullInput() throws Exception { + HTTPUtil.parseHeaders(null); + } + + private static void assertParseFailure(String input, String badLine) { + try { + HTTPUtil.parseHeaders(input); + fail("Expected an IOException"); + } catch (Exception e) { + assertTrue(e.getClass().getName(), e instanceof IOException); + assertEquals("Unable to parse header: " + badLine, e.getMessage()); + } + } +} From eba37019b250ad971fea95ea23c765b47ac9afcc Mon Sep 17 00:00:00 2001 From: Nico Piel Date: Thu, 29 Oct 2026 19:32:10 +0100 Subject: [PATCH 2/3] Parse headers in HTTPUtil without commons-httpclient 3 Replace HttpParser.parseHeaders with an equivalent inline parser so the userutil API no longer needs the EOL httpclient 3 jar. Line splitting, folding, trimming, UTF-8 decoding and the error message are unchanged; the parse error is now a java.net.ProtocolException (still an IOException) instead of the httpclient 3 ProtocolException. Signed-off-by: Nico Piel --- .../connect/server/userutil/HTTPUtil.java | 62 +++++++++++++++++-- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/mirth/connect/server/userutil/HTTPUtil.java b/server/src/main/java/com/mirth/connect/server/userutil/HTTPUtil.java index c03704bb7..69e6210a0 100644 --- a/server/src/main/java/com/mirth/connect/server/userutil/HTTPUtil.java +++ b/server/src/main/java/com/mirth/connect/server/userutil/HTTPUtil.java @@ -10,8 +10,11 @@ package com.mirth.connect.server.userutil; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.net.ProtocolException; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; @@ -21,8 +24,6 @@ import javax.xml.parsers.ParserConfigurationException; import org.apache.commons.fileupload.FileUploadBase; -import org.apache.commons.httpclient.Header; -import org.apache.commons.httpclient.HttpParser; import org.apache.commons.io.IOUtils; import org.apache.http.entity.ContentType; @@ -46,15 +47,66 @@ private HTTPUtil() {} */ public static Map parseHeaders(String str) throws Exception { Map headersMap = new HashMap(); - Header[] headers = HttpParser.parseHeaders(new ByteArrayInputStream(str.getBytes()), "UTF-8"); + InputStream is = new ByteArrayInputStream(str.getBytes()); + String name = null; + StringBuilder value = null; + + // Same rules as the commons-httpclient 3 HttpParser.parseHeaders this replaces + for (String line = readLine(is); line != null && line.trim().length() > 0; line = readLine(is)) { + if (line.charAt(0) == ' ' || line.charAt(0) == '\t') { + // Folded continuation line, appended to the previous header value + if (value != null) { + value.append(' ').append(line.trim()); + } + } else { + if (name != null) { + headersMap.put(name, value.toString()); + } + + int colon = line.indexOf(':'); + if (colon < 0) { + throw new ProtocolException("Unable to parse header: " + line); + } + name = line.substring(0, colon).trim(); + value = new StringBuilder(line.substring(colon + 1).trim()); + } + } - for (int i = 0; i < headers.length; i++) { - headersMap.put(headers[i].getName(), headers[i].getValue()); + if (name != null) { + headersMap.put(name, value.toString()); } return headersMap; } + /* + * Reads up to and including the next LF, strips a trailing LF or CRLF and decodes the line as + * UTF-8. Returns null at the end of the stream. + */ + private static String readLine(InputStream is) throws IOException { + ByteArrayOutputStream buf = new ByteArrayOutputStream(); + int ch; + while ((ch = is.read()) >= 0) { + buf.write(ch); + if (ch == '\n') { + break; + } + } + if (buf.size() == 0) { + return null; + } + + byte[] data = buf.toByteArray(); + int len = data.length; + if (data[len - 1] == '\n') { + len--; + if (len > 0 && data[len - 1] == '\r') { + len--; + } + } + return new String(data, 0, len, StandardCharsets.UTF_8); + } + /** * Serializes an HTTP request body into XML. Multipart requests will also automatically be * parsed into separate XML nodes. From 90c8e099edc899ce037221c64ddb29180195afc6 Mon Sep 17 00:00:00 2001 From: Nico Piel Date: Fri, 30 Oct 2026 16:43:33 +0100 Subject: [PATCH 3/3] Parse HTTPUtil headers with httpcore instead of a custom parser Signed-off-by: Nico Piel --- .../connect/server/userutil/HTTPUtil.java | 65 +++---------------- .../connect/server/userutil/HTTPUtilTest.java | 42 +++++++----- 2 files changed, 36 insertions(+), 71 deletions(-) diff --git a/server/src/main/java/com/mirth/connect/server/userutil/HTTPUtil.java b/server/src/main/java/com/mirth/connect/server/userutil/HTTPUtil.java index 69e6210a0..4919aeddc 100644 --- a/server/src/main/java/com/mirth/connect/server/userutil/HTTPUtil.java +++ b/server/src/main/java/com/mirth/connect/server/userutil/HTTPUtil.java @@ -10,10 +10,8 @@ package com.mirth.connect.server.userutil; import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.net.ProtocolException; import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; @@ -25,7 +23,11 @@ import org.apache.commons.fileupload.FileUploadBase; import org.apache.commons.io.IOUtils; +import org.apache.http.Header; import org.apache.http.entity.ContentType; +import org.apache.http.impl.io.AbstractMessageParser; +import org.apache.http.impl.io.HttpTransportMetricsImpl; +import org.apache.http.impl.io.SessionInputBufferImpl; import com.mirth.connect.connectors.http.HttpMessageConverter; import com.mirth.connect.donkey.util.DonkeyElement.DonkeyElementException; @@ -47,66 +49,17 @@ private HTTPUtil() {} */ public static Map parseHeaders(String str) throws Exception { Map headersMap = new HashMap(); - InputStream is = new ByteArrayInputStream(str.getBytes()); - String name = null; - StringBuilder value = null; - - // Same rules as the commons-httpclient 3 HttpParser.parseHeaders this replaces - for (String line = readLine(is); line != null && line.trim().length() > 0; line = readLine(is)) { - if (line.charAt(0) == ' ' || line.charAt(0) == '\t') { - // Folded continuation line, appended to the previous header value - if (value != null) { - value.append(' ').append(line.trim()); - } - } else { - if (name != null) { - headersMap.put(name, value.toString()); - } - - int colon = line.indexOf(':'); - if (colon < 0) { - throw new ProtocolException("Unable to parse header: " + line); - } - name = line.substring(0, colon).trim(); - value = new StringBuilder(line.substring(colon + 1).trim()); - } - } + SessionInputBufferImpl buffer = new SessionInputBufferImpl(new HttpTransportMetricsImpl(), 8192, 8192, null, StandardCharsets.UTF_8.newDecoder()); + buffer.bind(new ByteArrayInputStream(str.getBytes())); + Header[] headers = AbstractMessageParser.parseHeaders(buffer, -1, -1, null); - if (name != null) { - headersMap.put(name, value.toString()); + for (int i = 0; i < headers.length; i++) { + headersMap.put(headers[i].getName(), headers[i].getValue()); } return headersMap; } - /* - * Reads up to and including the next LF, strips a trailing LF or CRLF and decodes the line as - * UTF-8. Returns null at the end of the stream. - */ - private static String readLine(InputStream is) throws IOException { - ByteArrayOutputStream buf = new ByteArrayOutputStream(); - int ch; - while ((ch = is.read()) >= 0) { - buf.write(ch); - if (ch == '\n') { - break; - } - } - if (buf.size() == 0) { - return null; - } - - byte[] data = buf.toByteArray(); - int len = data.length; - if (data[len - 1] == '\n') { - len--; - if (len > 0 && data[len - 1] == '\r') { - len--; - } - } - return new String(data, 0, len, StandardCharsets.UTF_8); - } - /** * Serializes an HTTP request body into XML. Multipart requests will also automatically be * parsed into separate XML nodes. diff --git a/server/src/test/java/com/mirth/connect/server/userutil/HTTPUtilTest.java b/server/src/test/java/com/mirth/connect/server/userutil/HTTPUtilTest.java index c1d8936de..8b7a3b47e 100644 --- a/server/src/test/java/com/mirth/connect/server/userutil/HTTPUtilTest.java +++ b/server/src/test/java/com/mirth/connect/server/userutil/HTTPUtilTest.java @@ -8,17 +8,17 @@ import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; -import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; +import org.apache.http.ProtocolException; import org.junit.Test; /** * Characterization tests for {@link HTTPUtil#parseHeaders(String)}, pinning the behaviour of the - * original commons-httpclient 3 HttpParser based implementation. + * httpcore 4 AbstractMessageParser based implementation. */ public class HTTPUtilTest { @@ -48,7 +48,8 @@ public void testLfLinesAndNoTrailingNewline() throws Exception { @Test public void testNameAndValueAreTrimmed() throws Exception { assertEquals(map("Name", "some value"), HTTPUtil.parseHeaders("Name \t: \t some value \t \r\n")); - assertEquals(map("X", "a"), HTTPUtil.parseHeaders("X:\u000ba\u0001\r\n")); + // Only spaces and tabs are trimmed, other control characters are kept + assertEquals(map("X", "\u000ba\u0001"), HTTPUtil.parseHeaders("X:\u000ba\u0001\r\n")); } @Test @@ -57,23 +58,29 @@ public void testValueKeepsFurtherColons() throws Exception { } @Test - public void testEmptyNameAndEmptyValue() throws Exception { - assertEquals(map("", "v", "X", ""), HTTPUtil.parseHeaders(":v\r\nX:\r\n")); + public void testEmptyNameIsRejected() { + assertParseFailure(":v\r\nX:\r\n", ":v"); + } + + @Test + public void testEmptyValue() throws Exception { + assertEquals(map("X", ""), HTTPUtil.parseHeaders("X:\r\n")); } @Test public void testFoldedContinuationLines() throws Exception { - assertEquals(map("X", "a b c", "Y", "d"), HTTPUtil.parseHeaders("X: a\r\n b \r\n\tc\r\nY: d\r\n")); + // Leading whitespace of a folded line is dropped, trailing whitespace before the next fold is kept + assertEquals(map("X", "a b c", "Y", "d"), HTTPUtil.parseHeaders("X: a\r\n b \r\n\tc\r\nY: d\r\n")); } @Test public void testContinuationOfEmptyValue() throws Exception { - assertEquals(map("X", " b"), HTTPUtil.parseHeaders("X:\r\n b\r\n")); + assertEquals(map("X", "b"), HTTPUtil.parseHeaders("X:\r\n b\r\n")); } @Test - public void testLeadingContinuationLineIsIgnored() throws Exception { - assertEquals(map("X", "a"), HTTPUtil.parseHeaders(" orphan\r\nX: a\r\n")); + public void testLeadingContinuationLineIsRejected() { + assertParseFailure(" orphan\r\nX: a\r\n", " orphan"); } @Test @@ -93,9 +100,14 @@ public void testStopsAtBlankLine() throws Exception { } @Test - public void testStopsAtWhitespaceOnlyLine() throws Exception { - assertEquals(map("A", "1"), HTTPUtil.parseHeaders("A: 1\r\n \t \r\nB: 2\r\n")); - assertEquals(map("A", "1"), HTTPUtil.parseHeaders("A: 1\r\n\r\r\nB: 2\r\n")); + public void testWhitespaceOnlyLineIsFolded() throws Exception { + assertEquals(map("A", "1", "B", "2"), HTTPUtil.parseHeaders("A: 1\r\n \t \r\nB: 2\r\n")); + } + + @Test + public void testBareCarriageReturnLineIsRejected() { + // httpcore escapes the control character in the message + assertParseFailure("A: 1\r\n\r\r\nB: 2\r\n", "[0x10]"); } @Test @@ -134,10 +146,10 @@ public void testNullInput() throws Exception { private static void assertParseFailure(String input, String badLine) { try { HTTPUtil.parseHeaders(input); - fail("Expected an IOException"); + fail("Expected a ProtocolException"); } catch (Exception e) { - assertTrue(e.getClass().getName(), e instanceof IOException); - assertEquals("Unable to parse header: " + badLine, e.getMessage()); + assertTrue(e.getClass().getName(), e instanceof ProtocolException); + assertEquals("Invalid header: " + badLine, e.getMessage()); } } }