From d70fd979aed39ebbc88f3cc94c04b8c47f7960dc Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Wed, 16 Oct 2024 21:32:32 +0200 Subject: [PATCH] Add HTTP/1.1 TE header interceptor and strict client/server option Implemented a new `RequestTE` interceptor for validating the `TE` header in HTTP/1.1 requests, ensuring proper protocol handling. Introduced strict client and server options that allow users to enable additional protocol validation checks, such as the validation of `TE` headers for HTTP/2 and HTTP/1.1. Users can now choose between default or strict mode, depending on their performance and compliance needs. --- .../classic/ClassicIntegrationTest.java | 4 +- .../hc/core5/http/impl/HttpProcessors.java | 82 +++++++- .../hc/core5/http/protocol/RequestTE.java | 190 +++++++++++++++++ .../hc/core5/http/protocol/TestRequestTE.java | 199 ++++++++++++++++++ .../protocol/TestStandardInterceptors.java | 19 ++ 5 files changed, 491 insertions(+), 3 deletions(-) create mode 100644 httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestTE.java create mode 100644 httpcore5/src/test/java/org/apache/hc/core5/http/protocol/TestRequestTE.java diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java index e92582efd..22f27425c 100644 --- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java +++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java @@ -65,6 +65,7 @@ import org.apache.hc.core5.http.protocol.RequestConnControl; import org.apache.hc.core5.http.protocol.RequestContent; import org.apache.hc.core5.http.protocol.RequestExpectContinue; +import org.apache.hc.core5.http.protocol.RequestTE; import org.apache.hc.core5.http.protocol.RequestTargetHost; import org.apache.hc.core5.http.protocol.RequestUserAgent; import org.apache.hc.core5.testing.extension.classic.ClassicTestResources; @@ -638,7 +639,8 @@ void testHttpPostNoContentLength() throws Exception { RequestTargetHost.INSTANCE, RequestConnControl.INSTANCE, RequestUserAgent.INSTANCE, - RequestExpectContinue.INSTANCE)); + RequestExpectContinue.INSTANCE, + RequestTE.INSTANCE)); client.start(); final HttpCoreContext context = HttpCoreContext.create(); diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/HttpProcessors.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/HttpProcessors.java index 3da9d11dd..5a47dd0ff 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/HttpProcessors.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/HttpProcessors.java @@ -32,6 +32,7 @@ import org.apache.hc.core5.http.protocol.RequestConnControl; import org.apache.hc.core5.http.protocol.RequestContent; import org.apache.hc.core5.http.protocol.RequestExpectContinue; +import org.apache.hc.core5.http.protocol.RequestTE; import org.apache.hc.core5.http.protocol.RequestTargetHost; import org.apache.hc.core5.http.protocol.RequestUserAgent; import org.apache.hc.core5.http.protocol.RequestValidateHost; @@ -112,6 +113,43 @@ public static HttpProcessorBuilder customClient(final String agentInfo) { RequestExpectContinue.INSTANCE); } + /** + * Creates an {@link HttpProcessorBuilder} initialized with strict protocol interceptors + * for client-side HTTP/1.1 processing. + *

+ * This configuration enforces stricter validation and processing of client requests, + * ensuring compliance with the HTTP protocol. It includes interceptors for handling + * target hosts, content, connection controls, and TE header validation, among others. + * The user agent can be customized using the provided {@code agentInfo} parameter. + * + * @param agentInfo the user agent info to be included in the {@code User-Agent} header. + * If {@code null} or blank, a default value will be used. + * @return the {@link HttpProcessorBuilder} configured with strict client-side interceptors. + * @since 5.4 + */ + public static HttpProcessorBuilder strictClient(final String agentInfo) { + return HttpProcessorBuilder.create() + .addAll( + RequestTargetHost.INSTANCE, + RequestContent.INSTANCE, + RequestConnControl.INSTANCE, + RequestTE.INSTANCE, + new RequestUserAgent(!TextUtils.isBlank(agentInfo) ? agentInfo : + VersionInfo.getSoftwareInfo(SOFTWARE, "org.apache.hc.core5", HttpProcessors.class)), + RequestExpectContinue.INSTANCE); + } + + /** + * Creates {@link HttpProcessorBuilder} initialized with default protocol interceptors + * for client side HTTP/1.1 processing. + * + * @param agentInfo the agent info text or {@code null} for default. + * @return the processor builder. + */ + public static HttpProcessorBuilder customClient(final String agentInfo, final boolean strict) { + return strict ? strictClient(agentInfo) : customClient(agentInfo); + } + /** * Creates {@link HttpProcessor} initialized with default protocol interceptors * for client side HTTP/1.1 processing. @@ -120,7 +158,7 @@ public static HttpProcessorBuilder customClient(final String agentInfo) { * @return the processor. */ public static HttpProcessor client(final String agentInfo) { - return customClient(agentInfo).build(); + return client(agentInfo, false); } /** @@ -130,7 +168,47 @@ public static HttpProcessor client(final String agentInfo) { * @return the processor. */ public static HttpProcessor client() { - return customClient(null).build(); + return client(null); + } + + /** + * Creates an {@link HttpProcessor} for client-side HTTP/2 processing. + * This method allows the option to include strict protocol interceptors. + * + * @param agentInfo the agent info text or {@code null} for default. + * @param strict if {@code true}, strict protocol interceptors will be added, including the {@code TE} header validation. + * @return the configured HTTP processor. + * @since 5.4 + */ + public static HttpProcessor client(final String agentInfo, final boolean strict) { + return customClient(agentInfo, strict).build(); + } + + /** + * Creates an {@link HttpProcessor} for client-side HTTP/2 processing + * with strict protocol validation interceptors by default. + *

+ * Strict validation includes additional checks such as validating the {@code TE} header. + * + * @return the configured strict HTTP processor. + * @since 5.4 + */ + public static HttpProcessor clientStrict() { + return customClient(null, true).build(); + } + + /** + * Creates an {@link HttpProcessor} for client-side HTTP/2 processing + * with strict protocol validation interceptors, using the specified agent information. + *

+ * Strict validation includes additional checks such as validating the {@code TE} header. + * + * @param agentInfo the agent info text or {@code null} for default. + * @return the configured strict HTTP processor. + * @since 5.4 + */ + public static HttpProcessor clientStrict(final String agentInfo) { + return customClient(agentInfo, true).build(); } } diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestTE.java b/httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestTE.java new file mode 100644 index 000000000..bf8a453ae --- /dev/null +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestTE.java @@ -0,0 +1,190 @@ +/* + * ==================================================================== + * 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.protocol; + +import java.io.IOException; + +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.apache.hc.core5.http.EntityDetails; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HeaderElements; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpRequestInterceptor; +import org.apache.hc.core5.http.ProtocolException; +import org.apache.hc.core5.util.Args; +import org.apache.hc.core5.util.Tokenizer; + +/** + * HTTP request interceptor responsible for validating and processing the {@code TE} header field in HTTP/1.1 requests. + *

+ * The {@code TE} header is used to indicate transfer codings the client is willing to accept and, in some cases, whether + * the client is willing to accept trailer fields. This interceptor ensures that the {@code TE} header does not include + * the {@code chunked} transfer coding and validates the presence of the {@code Connection: TE} header. + *

+ * For HTTP/1.1 requests, the {@code TE} header can contain multiple values separated by commas and may include quality + * values (denoted by {@code q=}) separated by semicolons. + *

+ * In case of HTTP/2, this validation is skipped, and another layer of logic handles the specifics of HTTP/2 compliance. + * + * @since 5.4 + */ +@Contract(threading = ThreadingBehavior.IMMUTABLE) +public class RequestTE implements HttpRequestInterceptor { + + /** + * Singleton instance of the {@code RequestTE} interceptor. + */ + public static final HttpRequestInterceptor INSTANCE = new RequestTE(); + + /** + * Delimiter used to parse the {@code TE} header, recognizing both commas (',') and semicolons (';') as delimiters. + */ + public static final Tokenizer.Delimiter DELIMITER = Tokenizer.delimiters(',', ';'); + + /** + * Default constructor. + */ + public RequestTE() { + super(); + } + + /** + * Processes the {@code TE} header of the given HTTP request and ensures compliance with HTTP/1.1 requirements. + *

+ * If the {@code TE} header is present, this method validates that: + *

+ * + * @param request the HTTP request containing the headers to validate + * @param entity the entity associated with the request (may be {@code null}) + * @param context the execution context for the request + * @throws HttpException if the {@code TE} header contains invalid values or the {@code Connection} header is missing + * @throws IOException in case of an I/O error + */ + @Override + public void process(final HttpRequest request, final EntityDetails entity, final HttpContext context) + throws HttpException, IOException { + Args.notNull(request, "HTTP request"); + + // Fetch the TE header + final Header teHeader = request.getFirstHeader(HttpHeaders.TE); + + if (teHeader == null) { + return; // No further validation needed + } + + final String teValue = teHeader.getValue(); + validateTEField(teValue); + + validateConnectionHeader(request); + } + + /** + * Validates the {@code TE} header values for compliance with HTTP/1.1. + *

+ * Specifically, this method ensures that: + *

+ * + * @param teValue the value of the {@code TE} header + * @throws HttpException if the {@code TE} header contains invalid values + */ + private void validateTEField(final String teValue) throws HttpException { + final Tokenizer.Cursor cursor = new Tokenizer.Cursor(0, teValue.length()); + + while (!cursor.atEnd()) { + Tokenizer.INSTANCE.skipWhiteSpace(teValue, cursor); + + final String member = Tokenizer.INSTANCE.parseToken(teValue, cursor, DELIMITER); + + if (member.isEmpty()) { + if (!cursor.atEnd()) { + Tokenizer.INSTANCE.skipWhiteSpace(teValue, cursor); + cursor.updatePos(cursor.getPos() + 1); + } + continue; + } + + if ("trailers".equalsIgnoreCase(member)) { + continue; + } + + if (HeaderElements.CHUNKED_ENCODING.equalsIgnoreCase(member)) { + throw new ProtocolException("'chunked' transfer coding must not be listed in the TE header for HTTP/1.1."); + } + + if (!cursor.atEnd()) { + Tokenizer.INSTANCE.skipWhiteSpace(teValue, cursor); + cursor.updatePos(cursor.getPos() + 1); + } + } + } + + /** + * Validates the presence of the {@code Connection: TE} header when the {@code TE} header is present. + *

+ * If the {@code TE} header is used, the HTTP/1.1 protocol requires that the {@code Connection} header includes the {@code TE} directive to prevent forwarding by intermediaries. + * + * @param request the HTTP request to validate + * @throws HttpException if the {@code Connection: TE} header is missing + */ + private void validateConnectionHeader(final HttpRequest request) throws HttpException { + final Header connectionHeader = request.getFirstHeader(HttpHeaders.CONNECTION); + if (connectionHeader == null) { + throw new ProtocolException("The 'TE' header is present, but the 'Connection' header is missing."); + } + final String connectionValue = connectionHeader.getValue(); + final Tokenizer.Cursor cursor = new Tokenizer.Cursor(0, connectionValue.length()); + + boolean hasTE = false; + + while (!cursor.atEnd()) { + final String directive = Tokenizer.INSTANCE.parseToken(connectionValue, cursor, DELIMITER).trim(); + + if ("TE".equalsIgnoreCase(directive)) { + hasTE = true; + break; + } + + if (!cursor.atEnd()) { + cursor.updatePos(cursor.getPos() + 1); + } + } + + if (!hasTE) { + throw new ProtocolException("The 'Connection' header must include the 'TE' directive when the 'TE' header is present."); + } + } +} diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/protocol/TestRequestTE.java b/httpcore5/src/test/java/org/apache/hc/core5/http/protocol/TestRequestTE.java new file mode 100644 index 000000000..990c4909b --- /dev/null +++ b/httpcore5/src/test/java/org/apache/hc/core5/http/protocol/TestRequestTE.java @@ -0,0 +1,199 @@ +/* + * ==================================================================== + * 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.protocol; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpRequestInterceptor; +import org.apache.hc.core5.http.HttpVersion; +import org.apache.hc.core5.http.Method; +import org.apache.hc.core5.http.ProtocolException; +import org.apache.hc.core5.http.message.BasicClassicHttpRequest; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +class TestRequestTE { + + @Test + void testValidTEHeader() throws Exception { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + + // Set the TE header and Connection header + request.setHeader(HttpHeaders.TE, "trailers"); + request.setHeader(HttpHeaders.CONNECTION, "TE"); + + final HttpRequestInterceptor interceptor = new RequestTE(); + interceptor.process(request, request.getEntity(), context); + + assertNotNull(request.getHeader(HttpHeaders.TE)); + assertEquals("trailers", request.getHeader(HttpHeaders.TE).getValue()); + } + + + @Test + void testMultipleValidTEHeaders() throws Exception { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + + // Set both the TE header and the Connection header + request.setHeader(HttpHeaders.TE, "trailers, deflate;q=0.5"); + request.setHeader(HttpHeaders.CONNECTION, "TE"); + + final HttpRequestInterceptor interceptor = new RequestTE(); + interceptor.process(request, request.getEntity(), context); + + assertNotNull(request.getHeader(HttpHeaders.TE)); + assertEquals("trailers, deflate;q=0.5", request.getHeader(HttpHeaders.TE).getValue()); + } + + + @Test + void testTEHeaderNotPresent() throws Exception { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + + final HttpRequestInterceptor interceptor = new RequestTE(); + interceptor.process(request, request.getEntity(), context); + + // No TE header, no validation should occur + assertNull(request.getHeader(HttpHeaders.TE)); + } + + @Test + void testTEHeaderContainsChunked() { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + request.setHeader(HttpHeaders.TE, "chunked"); + + final HttpRequestInterceptor interceptor = new RequestTE(); + Assertions.assertThrows(ProtocolException.class, () -> + interceptor.process(request, request.getEntity(), context)); + } + + @Test + void testTEHeaderInvalidTransferCoding() { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + request.setHeader(HttpHeaders.TE, "invalid;q=abc"); + + final HttpRequestInterceptor interceptor = new RequestTE(); + Assertions.assertThrows(ProtocolException.class, () -> + interceptor.process(request, request.getEntity(), context)); + } + + @Test + void testTEHeaderAlreadySet() throws Exception { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + + final String teValue = "trailers"; + request.setHeader(HttpHeaders.TE, teValue); + request.setHeader(HttpHeaders.CONNECTION, "TE"); // Add the Connection header as required + + final HttpRequestInterceptor interceptor = new RequestTE(); + interceptor.process(request, request.getEntity(), context); + + assertEquals(HttpHeaders.TE, request.getHeader(HttpHeaders.TE).getName()); + assertNotNull(request.getHeader(HttpHeaders.TE)); + assertEquals(teValue, request.getHeader(HttpHeaders.TE).getValue()); + } + + + @Test + void testTEHeaderWithConnectionHeaderValidation() throws Exception { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + request.setHeader(HttpHeaders.TE, "trailers"); + request.setHeader(HttpHeaders.CONNECTION, "TE"); + + final HttpRequestInterceptor interceptor = new RequestTE(); + interceptor.process(request, request.getEntity(), context); + + assertEquals(HttpHeaders.TE, request.getHeader(HttpHeaders.TE).getName()); + assertNotNull(request.getHeader(HttpHeaders.TE)); + assertEquals("trailers", request.getHeader(HttpHeaders.TE).getValue()); + } + + @Test + void testTEHeaderWithoutConnectionHeaderThrowsException() { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + request.setHeader(HttpHeaders.TE, "trailers"); + + final HttpRequestInterceptor interceptor = new RequestTE(); + Assertions.assertThrows(ProtocolException.class, () -> + interceptor.process(request, request.getEntity(), context)); + } + + @Test + void testTEHeaderWithoutTEInConnectionHeaderThrowsException() { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + // Set TE header but Connection header does not include "TE" + request.setHeader(HttpHeaders.TE, "trailers"); + request.setHeader(HttpHeaders.CONNECTION, "keep-alive"); // Missing "TE" + + final HttpRequestInterceptor interceptor = new RequestTE(); + Assertions.assertThrows(ProtocolException.class, () -> + interceptor.process(request, request.getEntity(), context)); + } + + @Test + void testTEHeaderWithMultipleDirectivesInConnectionHeader() throws Exception { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + + // Set TE header and a Connection header with multiple directives + request.setHeader(HttpHeaders.TE, "trailers"); + request.setHeader(HttpHeaders.CONNECTION, "keep-alive, close, TE"); + + final HttpRequestInterceptor interceptor = new RequestTE(); + interceptor.process(request, request.getEntity(), context); + + assertNotNull(request.getHeader(HttpHeaders.CONNECTION)); + assertTrue(request.getHeader(HttpHeaders.CONNECTION).getValue().contains("TE")); + } + + +} + diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/protocol/TestStandardInterceptors.java b/httpcore5/src/test/java/org/apache/hc/core5/http/protocol/TestStandardInterceptors.java index bdf336a0d..e9f66b744 100644 --- a/httpcore5/src/test/java/org/apache/hc/core5/http/protocol/TestStandardInterceptors.java +++ b/httpcore5/src/test/java/org/apache/hc/core5/http/protocol/TestStandardInterceptors.java @@ -442,6 +442,25 @@ void testRequestTargetHostConnectHttp10() throws Exception { Assertions.assertNull(header); } + @Test + void testTEHeaderWithConnectionTE() throws Exception { + final HttpCoreContext context = HttpCoreContext.create(); + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + context.setProtocolVersion(HttpVersion.HTTP_1_1); + + // Set both TE and Connection headers as per the requirement + request.setHeader(HttpHeaders.TE, "trailers"); + request.setHeader(HttpHeaders.CONNECTION, "TE"); + + final RequestTE interceptor = new RequestTE(); + interceptor.process(request, request.getEntity(), context); + + final Header connectionHeader = request.getFirstHeader(HttpHeaders.CONNECTION); + Assertions.assertNotNull(connectionHeader); + Assertions.assertEquals("TE", connectionHeader.getValue()); + } + + @Test void testRequestUserAgentGenerated() throws Exception { final HttpCoreContext context = HttpCoreContext.create();