From 32da22f9d6d7fc733a26f010941c4f07ae702c8b Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Wed, 21 Oct 2026 10:45:57 +0200 Subject: [PATCH] Validate outgoing HTTP/1 header fields and trailers --- .../core5/http/impl/Http1FieldValidator.java | 87 +++++++++++++ .../http/impl/io/AbstractMessageWriter.java | 3 + .../http/impl/io/ChunkedOutputStream.java | 27 +++- .../http/impl/nio/AbstractMessageWriter.java | 3 + .../hc/core5/http/impl/nio/ChunkEncoder.java | 21 ++++ .../impl/io/TestHttp1FieldValidation.java | 119 ++++++++++++++++++ .../impl/nio/TestHttp1FieldValidation.java | 118 +++++++++++++++++ 7 files changed, 375 insertions(+), 3 deletions(-) create mode 100644 httpcore5/src/main/java/org/apache/hc/core5/http/impl/Http1FieldValidator.java create mode 100644 httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestHttp1FieldValidation.java create mode 100644 httpcore5/src/test/java/org/apache/hc/core5/http/impl/nio/TestHttp1FieldValidation.java diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/Http1FieldValidator.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/Http1FieldValidator.java new file mode 100644 index 0000000000..746a509c28 --- /dev/null +++ b/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 + * . + * + */ +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; + } + } +} diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/AbstractMessageWriter.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/AbstractMessageWriter.java index 6153aec3b8..9c29f18848 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/AbstractMessageWriter.java +++ b/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); } } diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/ChunkedOutputStream.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/ChunkedOutputStream.java index b7d9faf4cc..009ec59c6d 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/ChunkedOutputStream.java +++ b/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 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 trailers = this.trailerSupplier != null ? this.trailerSupplier.get() : null; + private void validateTrailers(final List 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 trailers) throws IOException { if (trailers != null) { for (int i = 0; i < trailers.size(); i++) { final Header header = trailers.get(i); diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractMessageWriter.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractMessageWriter.java index 16bd77f022..53b5feb11e 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractMessageWriter.java +++ b/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); } } diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/ChunkEncoder.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/ChunkEncoder.java index 223f6fbc79..60480989e0 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/ChunkEncoder.java +++ b/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 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 trailers) throws IOException { super.complete(trailers); } + private void validateTrailers(final List 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 trailers) throws IOException { if (trailers != null) { for (int i = 0; i < trailers.size(); i++) { diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestHttp1FieldValidation.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestHttp1FieldValidation.java new file mode 100644 index 0000000000..13d999e039 --- /dev/null +++ b/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 + * . + * + */ +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); + } +} diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/nio/TestHttp1FieldValidation.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/nio/TestHttp1FieldValidation.java new file mode 100644 index 0000000000..8c015ef320 --- /dev/null +++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/nio/TestHttp1FieldValidation.java @@ -0,0 +1,118 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.core5.http.impl.nio; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.channels.Channels; +import java.nio.charset.StandardCharsets; +import java.util.Collections; + +import org.apache.hc.core5.http.ProtocolException; +import org.apache.hc.core5.http.impl.BasicHttpTransportMetrics; +import org.apache.hc.core5.http.message.BasicHeader; +import org.apache.hc.core5.http.message.BasicHttpRequest; +import org.apache.hc.core5.http.message.BasicHttpResponse; +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 BasicHttpRequest request = new BasicHttpRequest("GET", "/"); + request.addHeader(BufferedHeader.create(original)); + final SessionOutputBufferImpl output = new SessionOutputBufferImpl( + 256, 256, StandardCharsets.UTF_8.newEncoder()); + Assertions.assertThrows(ProtocolException.class, + () -> new DefaultHttpRequestWriter<>().write(request, output)); + Assertions.assertEquals("X-Test: ok\r\nX-Injected: true", original.toString()); + } + + @Test + void testRejectsInvalidNameAndNul() { + final BasicHttpRequest badName = new BasicHttpRequest("GET", "/"); + badName.addHeader(new BasicHeader("X-Test\r\nX-Injected", "true")); + Assertions.assertThrows(ProtocolException.class, + () -> new DefaultHttpRequestWriter<>().write(badName, + new SessionOutputBufferImpl(256, 256, StandardCharsets.UTF_8.newEncoder()))); + + final BasicHttpRequest nul = new BasicHttpRequest("GET", "/"); + nul.addHeader(new BasicHeader("X-Test", "a\0b")); + Assertions.assertThrows(ProtocolException.class, + () -> new DefaultHttpRequestWriter<>().write(nul, + new SessionOutputBufferImpl(256, 256, StandardCharsets.UTF_8.newEncoder()))); + } + + @Test + void testRejectsResponseHeader() { + final BasicHttpResponse response = new BasicHttpResponse(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, 256, StandardCharsets.UTF_8.newEncoder()))); + } + + @Test + void testRejectsTrailerBeforeLastChunk() { + final SessionOutputBufferImpl output = new SessionOutputBufferImpl( + 256, 256, StandardCharsets.UTF_8.newEncoder()); + final ChunkEncoder encoder = new ChunkEncoder( + Channels.newChannel(new ByteArrayOutputStream()), output, new BasicHttpTransportMetrics()); + final IOException ex = Assertions.assertThrows(IOException.class, + () -> encoder.complete(Collections.singletonList(BufferedHeader.create( + line("X-Trailer: ok\r\nX-Injected: true"))))); + Assertions.assertInstanceOf(ProtocolException.class, ex.getCause()); + Assertions.assertEquals(0, output.length(), "No closing chunk must be buffered"); + } + + @Test + void testValidFieldsPreserveWireFormat() throws Exception { + final BasicHttpRequest request = new BasicHttpRequest("GET", "/"); + request.addHeader(BufferedHeader.create(line("X-Raw: original formatting"))); + request.addHeader(new BasicHeader("X-Normal", "abc\r\ndef")); + final SessionOutputBufferImpl output = new SessionOutputBufferImpl( + 256, 256, StandardCharsets.UTF_8.newEncoder()); + new DefaultHttpRequestWriter<>().write(request, output); + final ByteArrayOutputStream wire = new ByteArrayOutputStream(); + while (output.hasData()) { + output.flush(Channels.newChannel(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); + } +}