From 857d16215ec6211cdeae917012e9204db89c0351 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Wed, 21 Oct 2026 13:12:40 +0200 Subject: [PATCH] Filter control characters in encoded HTTP/1 output lines --- .../http/impl/io/SessionOutputBufferImpl.java | 17 +++++- .../impl/nio/SessionOutputBufferImpl.java | 6 +- .../org/apache/hc/core5/util/TextUtils.java | 17 +++++- .../http/impl/io/TestSessionInOutBuffers.java | 47 +++++++++++++++ .../impl/nio/TestSessionInOutBuffers.java | 59 +++++++++++++++++++ .../apache/hc/core5/util/TestTextUtils.java | 11 ++++ 6 files changed, 152 insertions(+), 5 deletions(-) diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/SessionOutputBufferImpl.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/SessionOutputBufferImpl.java index c186f0041b..d5ebd25381 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/SessionOutputBufferImpl.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/SessionOutputBufferImpl.java @@ -33,6 +33,7 @@ import java.nio.CharBuffer; import java.nio.charset.CharsetEncoder; import java.nio.charset.CoderResult; +import java.util.Arrays; import org.apache.hc.core5.http.Chars; import org.apache.hc.core5.http.impl.BasicHttpTransportMetrics; @@ -41,6 +42,7 @@ import org.apache.hc.core5.util.Args; import org.apache.hc.core5.util.ByteArrayBuffer; import org.apache.hc.core5.util.CharArrayBuffer; +import org.apache.hc.core5.util.TextUtils; /** * Abstract base class for session output buffers that stream data to @@ -207,7 +209,20 @@ public void writeLine(final CharArrayBuffer charbuffer, final OutputStream outpu remaining -= chunk; } } else { - final CharBuffer cbuf = CharBuffer.wrap(charbuffer.array(), 0, charbuffer.length()); + // Filter control characters that bypass the default castAsByte conversion. + final char[] chars = charbuffer.array(); + final int length = charbuffer.length(); + char[] sanitized = null; + for (int i = 0; i < length; i++) { + final char ch = chars[i]; + if (!TextUtils.isPrintable(ch)) { + if (sanitized == null) { + sanitized = Arrays.copyOf(chars, length); + } + sanitized[i] = '?'; + } + } + final CharBuffer cbuf = CharBuffer.wrap(sanitized != null ? sanitized : chars, 0, length); writeEncoded(cbuf, outputStream); } write(CRLF, outputStream); diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/SessionOutputBufferImpl.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/SessionOutputBufferImpl.java index 0ef5611fda..c9382e6e25 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/SessionOutputBufferImpl.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/SessionOutputBufferImpl.java @@ -198,7 +198,11 @@ public void writeLine(final CharArrayBuffer lineBuffer) throws CharacterCodingEx // terminate the encoding process eol = true; } - this.charbuffer.put(lineBuffer.array(), offset, l); + // Filter control characters while copying into the encoder buffer. + for (int i = 0; i < l; i++) { + final char ch = lineBuffer.charAt(offset + i); + this.charbuffer.put(TextUtils.isPrintable(ch) ? ch : '?'); + } this.charbuffer.flip(); boolean retry = true; diff --git a/httpcore5/src/main/java/org/apache/hc/core5/util/TextUtils.java b/httpcore5/src/main/java/org/apache/hc/core5/util/TextUtils.java index e36ab710d3..1fa8922bf6 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/util/TextUtils.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/util/TextUtils.java @@ -180,9 +180,7 @@ public static boolean isAllASCII(final CharSequence s) { */ @Internal public static byte castAsByte(final int c) { - if (c >= 0x20 && c <= 0x7E || // Visible ASCII - c >= 0xA0 && c <= 0xFF || // Visible ISO-8859-1 - c == 0x09) { // TAB + if (c <= 0xFF && isPrintable(c)) { return (byte) c; } return '?'; @@ -204,5 +202,18 @@ public static boolean isHex(final char c) { || c >= 'a' && c <= 'f'; } + /** + * Tests whether a character can be written to an HTTP protocol line without + * replacing it. Horizontal tab is permitted for compatibility. + * + * @param c The character to test. + * @return {@code true} if the character does not need filtering. + * @since 5.5 + */ + @Internal + public static boolean isPrintable(final int c) { + return c == 0x09 || c >= 0x20 && (c < 0x7F || c > 0x9F); + } + } diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestSessionInOutBuffers.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestSessionInOutBuffers.java index 22444d811f..0620051d85 100644 --- a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestSessionInOutBuffers.java +++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestSessionInOutBuffers.java @@ -50,6 +50,53 @@ class TestSessionInOutBuffers { + @Test + void testEncodedWriteLineFiltersControlCharacters() throws Exception { + final String original = "X-Test: ok\r\nX-Injected: yes\0\u001f\u007f\u0085\t\u00e9\u20ac"; + final CharArrayBuffer line = new CharArrayBuffer(original.length()); + line.append(original); + final SessionOutputBuffer outbuffer = new SessionOutputBufferImpl(8, StandardCharsets.UTF_8.newEncoder()); + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + + outbuffer.writeLine(line, out); + outbuffer.flush(out); + + Assertions.assertEquals("X-Test: ok??X-Injected: yes????\t\u00e9\u20ac\r\n", + new String(out.toByteArray(), StandardCharsets.UTF_8)); + Assertions.assertEquals(original, line.toString()); + } + + @Test + void testDefaultWriteLineFilteringIsUnchanged() throws Exception { + final CharArrayBuffer line = new CharArrayBuffer(32); + line.append("X-Test: ok\r\nX-Injected: yes"); + final SessionOutputBuffer outbuffer = new SessionOutputBufferImpl(8); + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + + outbuffer.writeLine(line, out); + outbuffer.flush(out); + + Assertions.assertEquals("X-Test: ok??X-Injected: yes\r\n", + new String(out.toByteArray(), StandardCharsets.ISO_8859_1)); + } + + @Test + void testWriteBinaryDataWithCharsetEncoder() throws Exception { + final byte[] payload = { + 0x00, '\r', '\n', '\t', 0x1f, 0x7f, + (byte) 0x80, (byte) 0xff + }; + + final SessionOutputBuffer outbuffer = new SessionOutputBufferImpl( + 16, StandardCharsets.UTF_8.newEncoder()); + final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + + outbuffer.write(payload, outputStream); + outbuffer.flush(outputStream); + + Assertions.assertArrayEquals(payload, outputStream.toByteArray()); + } + @Test void testBasicBufferProperties() throws Exception { final SessionInputBuffer inBuffer = new SessionInputBufferImpl(16); diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/nio/TestSessionInOutBuffers.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/nio/TestSessionInOutBuffers.java index 9344c0f4e3..7f0f47c299 100644 --- a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/nio/TestSessionInOutBuffers.java +++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/nio/TestSessionInOutBuffers.java @@ -68,6 +68,65 @@ private static ReadableByteChannel newChannel(final String s) { return newChannel(s, StandardCharsets.US_ASCII); } + @Test + void testEncodedWriteLineFiltersControlCharacters() throws Exception { + final String original = "X-Test: ok\r\nX-Injected: yes\0\u001f\u007f\u0085\t\u00e9\u20ac"; + final CharArrayBuffer line = new CharArrayBuffer(original.length()); + line.append(original); + final SessionOutputBuffer outbuffer = new SessionOutputBufferImpl( + 16, 8, StandardCharsets.UTF_8.newEncoder()); + + outbuffer.writeLine(line); + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final WritableByteChannel channel = newChannel(out); + while (outbuffer.hasData()) { + outbuffer.flush(channel); + } + + Assertions.assertEquals("X-Test: ok??X-Injected: yes????\t\u00e9\u20ac\r\n", + new String(out.toByteArray(), StandardCharsets.UTF_8)); + Assertions.assertEquals(original, line.toString()); + } + + @Test + void testDefaultWriteLineFilteringIsUnchanged() throws Exception { + final CharArrayBuffer line = new CharArrayBuffer(32); + line.append("X-Test: ok\r\nX-Injected: yes"); + final SessionOutputBuffer outbuffer = new SessionOutputBufferImpl(16, 8); + + outbuffer.writeLine(line); + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final WritableByteChannel channel = newChannel(out); + while (outbuffer.hasData()) { + outbuffer.flush(channel); + } + + Assertions.assertEquals("X-Test: ok??X-Injected: yes\r\n", + new String(out.toByteArray(), StandardCharsets.ISO_8859_1)); + } + + @Test + void testWriteBinaryDataWithCharsetEncoder() throws Exception { + final byte[] payload = { + 0x00, '\r', '\n', '\t', 0x1f, 0x7f, + (byte) 0x80, (byte) 0xff + }; + + final SessionOutputBuffer outbuffer = new SessionOutputBufferImpl( + 16, 16, StandardCharsets.UTF_8.newEncoder()); + + outbuffer.write(ByteBuffer.wrap(payload)); + + final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + final WritableByteChannel channel = Channels.newChannel(outputStream); + + while (outbuffer.hasData()) { + Assertions.assertTrue(outbuffer.flush(channel) > 0); + } + + Assertions.assertArrayEquals(payload, outputStream.toByteArray()); + } + @Test void testReadLineChunks() throws Exception { final SessionInputBuffer inbuf = new SessionInputBufferImpl(16, 16); diff --git a/httpcore5/src/test/java/org/apache/hc/core5/util/TestTextUtils.java b/httpcore5/src/test/java/org/apache/hc/core5/util/TestTextUtils.java index 67cda2d25d..0c67cc4ebb 100644 --- a/httpcore5/src/test/java/org/apache/hc/core5/util/TestTextUtils.java +++ b/httpcore5/src/test/java/org/apache/hc/core5/util/TestTextUtils.java @@ -67,4 +67,15 @@ void testToHexString() { Assertions.assertNull(TextUtils.toHexString(null)); } + @Test + void testPrintableCharactersAndByteConversion() { + for (int c = 0; c <= 0xFF; c++) { + final boolean expected = c == '\t' || c >= 0x20 && c <= 0x7E || c >= 0xA0; + Assertions.assertEquals(expected, TextUtils.isPrintable(c), "Character: " + c); + Assertions.assertEquals(expected ? (byte) c : (byte) '?', TextUtils.castAsByte(c), "Character: " + c); + } + Assertions.assertTrue(TextUtils.isPrintable(0x20AC)); + Assertions.assertEquals((byte) '?', TextUtils.castAsByte(0x20AC)); + } + }