Reject malformed HTTP/1 header fields and trailers in both classic and async transports.

httpcore5/src/main/java/org/apache/hc/core5/http/impl/Http1FieldValidator.java

@@ -0,0 +1,87 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.core5.http.impl;
+
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.http.ProtocolException;
+import org.apache.hc.core5.util.CharArrayBuffer;
+
+/**
+ * Validates serialized HTTP/1 field lines without modifying their content.
+ */
+@Internal
+public final class Http1FieldValidator {
+
+    private Http1FieldValidator() {
+    }
+
+    public static void validate(final CharArrayBuffer line) throws ProtocolException {
+        final int colon = line.indexOf(':');
+        if (colon <= 0) {
+            throw new ProtocolException("Invalid HTTP/1 field name");
+        }
+        for (int i = 0; i < colon; i++) {
+            if (!isTokenChar(line.charAt(i))) {
+                throw new ProtocolException("Invalid HTTP/1 field name");
+            }
+        }
+        for (int i = colon + 1; i < line.length(); i++) {
+            final char ch = line.charAt(i);
+            if (ch < 0x20 && ch != '\t' || ch == 0x7f) {
+                throw new ProtocolException("Invalid HTTP/1 field value");
+            }
+        }
+    }
+
+    private static boolean isTokenChar(final char ch) {
+        if (ch >= '0' && ch <= '9'
+                || ch >= 'A' && ch <= 'Z'
+                || ch >= 'a' && ch <= 'z') {
+            return true;
+        }
+        switch (ch) {
+            case '!':
+            case '#':
+            case '$':
+            case '%':
+            case '&':
+            case '\'':
+            case '*':
+            case '+':
+            case '-':
+            case '.':
+            case '^':
+            case '_':
+            case '`':
+            case '|':
+            case '~':
+                return true;
+            default:
+                return false;
+        }
+    }
+}

httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/AbstractMessageWriter.java

@@ -35,6 +35,7 @@
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpMessage;
+import org.apache.hc.core5.http.impl.Http1FieldValidator;
 import org.apache.hc.core5.http.io.HttpMessageWriter;
 import org.apache.hc.core5.http.io.SessionOutputBuffer;
 import org.apache.hc.core5.http.message.BasicLineFormatter;
@@ -93,10 +94,12 @@ public void write(final T message, final SessionOutputBuffer buffer, final Outpu
             final Header header = it.next();
             if (header instanceof FormattedHeader) {
                 final CharArrayBuffer chbuffer = ((FormattedHeader) header).getBuffer();
+                Http1FieldValidator.validate(chbuffer);
                 buffer.writeLine(chbuffer, outputStream);
             } else {
                 this.lineBuf.clear();
                 lineFormatter.formatHeader(this.lineBuf, header);
+                Http1FieldValidator.validate(this.lineBuf);
                 buffer.writeLine(this.lineBuf, outputStream);
             }
         }

httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/ChunkedOutputStream.java

@@ -34,6 +34,8 @@
 import org.apache.hc.core5.function.Supplier;
 import org.apache.hc.core5.http.FormattedHeader;
 import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.ProtocolException;
+import org.apache.hc.core5.http.impl.Http1FieldValidator;
 import org.apache.hc.core5.http.StreamClosedException;
 import org.apache.hc.core5.http.io.SessionOutputBuffer;
 import org.apache.hc.core5.http.message.BasicLineFormatter;
@@ -147,17 +149,36 @@ private void flushCacheWithAppend(final byte[] bufferToAppend, final int off, fi
     }
 
     private void writeClosingChunk() throws IOException {
+        final List<? extends Header> trailers = this.trailerSupplier != null ? this.trailerSupplier.get() : null;
+        validateTrailers(trailers);
         // Write the final chunk.
         this.lineBuffer.clear();
         this.lineBuffer.append('0');
         this.buffer.writeLine(this.lineBuffer, this.outputStream);
-        writeTrailers();
+        writeTrailers(trailers);
         this.lineBuffer.clear();
         this.buffer.writeLine(this.lineBuffer, this.outputStream);
     }
 
-    private void writeTrailers() throws IOException {
-        final List<? extends Header> trailers = this.trailerSupplier != null ? this.trailerSupplier.get() : null;
+    private void validateTrailers(final List<? extends Header> trailers) throws IOException {
+        if (trailers != null) {
+            try {
+                for (final Header header : trailers) {
+                    this.lineBuffer.clear();
+                    if (header instanceof FormattedHeader) {
+                        Http1FieldValidator.validate(((FormattedHeader) header).getBuffer());
+                    } else {
+                        BasicLineFormatter.INSTANCE.formatHeader(this.lineBuffer, header);
+                        Http1FieldValidator.validate(this.lineBuffer);
+                    }
+                }
+            } catch (final ProtocolException ex) {
+                throw new IOException(ex.getMessage(), ex);
+            }
+        }
+    }
+
+    private void writeTrailers(final List<? extends Header> trailers) throws IOException {
         if (trailers != null) {
             for (int i = 0; i < trailers.size(); i++) {
                 final Header header = trailers.get(i);

httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractMessageWriter.java

@@ -34,6 +34,7 @@
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpMessage;
+import org.apache.hc.core5.http.impl.Http1FieldValidator;
 import org.apache.hc.core5.http.message.BasicLineFormatter;
 import org.apache.hc.core5.http.message.LineFormatter;
 import org.apache.hc.core5.http.nio.NHttpMessageWriter;
@@ -94,10 +95,12 @@ public void write(final T message, final SessionOutputBuffer sessionBuffer) thro
             final Header header = it.next();
             if (header instanceof FormattedHeader) {
                 final CharArrayBuffer buffer = ((FormattedHeader) header).getBuffer();
+                Http1FieldValidator.validate(buffer);
                 sessionBuffer.writeLine(buffer);
             } else {
                 this.lineBuf.clear();
                 this.lineFormatter.formatHeader(this.lineBuf, header);
+                Http1FieldValidator.validate(this.lineBuf);
                 sessionBuffer.writeLine(this.lineBuf);
             }
         }

httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/ChunkEncoder.java

@@ -34,7 +34,9 @@
 
 import org.apache.hc.core5.http.FormattedHeader;
 import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.ProtocolException;
 import org.apache.hc.core5.http.impl.BasicHttpTransportMetrics;
+import org.apache.hc.core5.http.impl.Http1FieldValidator;
 import org.apache.hc.core5.http.message.BasicLineFormatter;
 import org.apache.hc.core5.http.nio.SessionOutputBuffer;
 import org.apache.hc.core5.util.CharArrayBuffer;
@@ -126,6 +128,7 @@ public int write(final ByteBuffer src) throws IOException {
     @Override
     public void complete(final List<? extends Header> trailers) throws IOException {
         assertNotCompleted();
+        validateTrailers(trailers);
         this.lineBuffer.clear();
         this.lineBuffer.append("0");
         this.buffer.writeLine(this.lineBuffer);
@@ -135,6 +138,24 @@ public void complete(final List<? extends Header> trailers) throws IOException {
         super.complete(trailers);
     }
 
+    private void validateTrailers(final List<? extends Header> trailers) throws IOException {
+        if (trailers != null) {
+            try {
+                for (final Header header : trailers) {
+                    this.lineBuffer.clear();
+                    if (header instanceof FormattedHeader) {
+                        Http1FieldValidator.validate(((FormattedHeader) header).getBuffer());
+                    } else {
+                        BasicLineFormatter.INSTANCE.formatHeader(this.lineBuffer, header);
+                        Http1FieldValidator.validate(this.lineBuffer);
+                    }
+                }
+            } catch (final ProtocolException ex) {
+                throw new IOException(ex.getMessage(), ex);
+            }
+        }
+    }
+
     private void writeTrailers(final List<? extends Header> trailers) throws IOException {
         if (trailers != null) {
             for (int i = 0; i < trailers.size(); i++) {

httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestHttp1FieldValidation.java

@@ -0,0 +1,119 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.core5.http.impl.io;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+
+import org.apache.hc.core5.http.ProtocolException;
+import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
+import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
+import org.apache.hc.core5.http.message.BasicHeader;
+import org.apache.hc.core5.http.message.BufferedHeader;
+import org.apache.hc.core5.util.CharArrayBuffer;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class TestHttp1FieldValidation {
+
+    private static CharArrayBuffer line(final String text) {
+        final CharArrayBuffer buffer = new CharArrayBuffer(text.length());
+        buffer.append(text);
+        return buffer;
+    }
+
+    @Test
+    void testRejectsRawCRLFWithUtf8Encoder() {
+        final CharArrayBuffer original = line("X-Test: ok\r\nX-Injected: true");
+        final BasicClassicHttpRequest request = new BasicClassicHttpRequest("GET", "/");
+        request.addHeader(BufferedHeader.create(original));
+        final ByteArrayOutputStream wire = new ByteArrayOutputStream();
+        final SessionOutputBufferImpl output = new SessionOutputBufferImpl(
+                256, StandardCharsets.UTF_8.newEncoder());
+
+        Assertions.assertThrows(ProtocolException.class,
+                () -> new DefaultHttpRequestWriter().write(request, output, wire));
+        Assertions.assertEquals("X-Test: ok\r\nX-Injected: true", original.toString());
+    }
+
+    @Test
+    void testRejectsInvalidNameAndNul() {
+        final BasicClassicHttpRequest badName = new BasicClassicHttpRequest("GET", "/");
+        badName.addHeader(new BasicHeader("X-Test\r\nX-Injected", "true"));
+        Assertions.assertThrows(ProtocolException.class,
+                () -> new DefaultHttpRequestWriter().write(badName,
+                        new SessionOutputBufferImpl(256, StandardCharsets.UTF_8.newEncoder()),
+                        new ByteArrayOutputStream()));
+
+        final BasicClassicHttpRequest nul = new BasicClassicHttpRequest("GET", "/");
+        nul.addHeader(new BasicHeader("X-Test", "a\0b"));
+        Assertions.assertThrows(ProtocolException.class,
+                () -> new DefaultHttpRequestWriter().write(nul,
+                        new SessionOutputBufferImpl(256, StandardCharsets.UTF_8.newEncoder()),
+                        new ByteArrayOutputStream()));
+    }
+
+    @Test
+    void testRejectsResponseHeader() {
+        final BasicClassicHttpResponse response = new BasicClassicHttpResponse(200);
+        response.addHeader(BufferedHeader.create(line("Set-Cookie: a=b\r\nX-Injected: true")));
+        Assertions.assertThrows(ProtocolException.class,
+                () -> new DefaultHttpResponseWriter().write(response,
+                        new SessionOutputBufferImpl(256, StandardCharsets.UTF_8.newEncoder()),
+                        new ByteArrayOutputStream()));
+    }
+
+    @Test
+    void testRejectsTrailerBeforeLastChunk() {
+        final SessionOutputBufferImpl output = new SessionOutputBufferImpl(
+                256, StandardCharsets.UTF_8.newEncoder());
+        final ByteArrayOutputStream wire = new ByteArrayOutputStream();
+        final ChunkedOutputStream stream = new ChunkedOutputStream(output, wire, 128,
+                () -> Collections.singletonList(BufferedHeader.create(
+                        line("X-Trailer: ok\r\nX-Injected: true"))));
+        final IOException ex = Assertions.assertThrows(IOException.class, stream::finish);
+        Assertions.assertInstanceOf(ProtocolException.class, ex.getCause());
+        Assertions.assertEquals(0, output.length(), "No closing chunk must be buffered");
+    }
+
+    @Test
+    void testValidFieldsPreserveWireFormat() throws Exception {
+        final BasicClassicHttpRequest request = new BasicClassicHttpRequest("GET", "/");
+        request.addHeader(BufferedHeader.create(line("X-Raw:   original   formatting")));
+        request.addHeader(new BasicHeader("X-Normal", "abc\r\ndef"));
+        final ByteArrayOutputStream wire = new ByteArrayOutputStream();
+        final SessionOutputBufferImpl output = new SessionOutputBufferImpl(
+                256, StandardCharsets.UTF_8.newEncoder());
+        new DefaultHttpRequestWriter().write(request, output, wire);
+        output.flush(wire);
+        final String serialized = new String(wire.toByteArray(), StandardCharsets.ISO_8859_1);
+        Assertions.assertTrue(serialized.contains("\r\nX-Raw:   original   formatting\r\n"), serialized);
+        Assertions.assertTrue(serialized.contains("\r\nX-Normal: abc  def\r\n"), serialized);
+    }
+}