Filter control characters in encoded HTTP/1 output lines

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);

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;

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);
+    }
+
 
 }

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);

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);

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));
+    }
+
 }