Skip to content

Commit

Permalink
Bug fix: move H2 illegal message header check from message converters…
Browse files Browse the repository at this point in the history
… to protocol interceptors
  • Loading branch information
ok2c committed Oct 12, 2024
1 parent d43f13a commit 549e239
Show file tree
Hide file tree
Showing 8 changed files with 302 additions and 168 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

import org.apache.hc.core5.http.Header;
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.HttpVersion;
import org.apache.hc.core5.http.Method;
Expand Down Expand Up @@ -107,14 +106,6 @@ public HttpRequest convert(final List<Header> headers) throws HttpException {
throw new ProtocolException("Unsupported request header '%s'", name);
}
} else {
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION) || name.equalsIgnoreCase(HttpHeaders.KEEP_ALIVE)
|| name.equalsIgnoreCase(HttpHeaders.PROXY_CONNECTION) || name.equalsIgnoreCase(HttpHeaders.TRANSFER_ENCODING)
|| name.equalsIgnoreCase(HttpHeaders.HOST) || name.equalsIgnoreCase(HttpHeaders.UPGRADE)) {
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
}
if (name.equalsIgnoreCase(HttpHeaders.TE) && !value.equalsIgnoreCase("trailers")) {
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
}
messageHeaders.add(header);
}
}
Expand Down Expand Up @@ -196,14 +187,6 @@ public List<Header> convert(final HttpRequest message) throws HttpException {
if (name.startsWith(":")) {
throw new ProtocolException("Header name '%s' is invalid", name);
}
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION) || name.equalsIgnoreCase(HttpHeaders.KEEP_ALIVE)
|| name.equalsIgnoreCase(HttpHeaders.PROXY_CONNECTION) || name.equalsIgnoreCase(HttpHeaders.TRANSFER_ENCODING)
|| name.equalsIgnoreCase(HttpHeaders.HOST) || name.equalsIgnoreCase(HttpHeaders.UPGRADE)) {
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
}
if (name.equalsIgnoreCase(HttpHeaders.TE) && !value.equalsIgnoreCase("trailers")) {
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
}
headers.add(new BasicHeader(TextUtils.toLowerCase(name), value));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@

import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.ProtocolException;
Expand Down Expand Up @@ -82,10 +81,6 @@ public HttpResponse convert(final List<Header> headers) throws HttpException {
throw new ProtocolException("Unsupported response header '%s'", name);
}
} else {
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION) || name.equalsIgnoreCase(HttpHeaders.KEEP_ALIVE)
|| name.equalsIgnoreCase(HttpHeaders.TRANSFER_ENCODING) || name.equalsIgnoreCase(HttpHeaders.UPGRADE)) {
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
}
messageHeaders.add(header);
}

Expand Down Expand Up @@ -124,10 +119,6 @@ public List<Header> convert(final HttpResponse message) throws HttpException {
if (name.startsWith(":")) {
throw new ProtocolException("Header name '%s' is invalid", name);
}
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION) || name.equalsIgnoreCase(HttpHeaders.KEEP_ALIVE)
|| name.equalsIgnoreCase(HttpHeaders.TRANSFER_ENCODING) || name.equalsIgnoreCase(HttpHeaders.UPGRADE)) {
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
}
headers.add(new BasicHeader(TextUtils.toLowerCase(name), value));
}
return headers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@
import org.apache.hc.core5.http.protocol.ResponseConformance;
import org.apache.hc.core5.http.protocol.ResponseDate;
import org.apache.hc.core5.http.protocol.ResponseServer;
import org.apache.hc.core5.http2.protocol.H2RequestConformance;
import org.apache.hc.core5.http2.protocol.H2RequestConnControl;
import org.apache.hc.core5.http2.protocol.H2RequestContent;
import org.apache.hc.core5.http2.protocol.H2RequestTargetHost;
import org.apache.hc.core5.http2.protocol.H2RequestValidateHost;
import org.apache.hc.core5.http2.protocol.H2ResponseConformance;
import org.apache.hc.core5.http2.protocol.H2ResponseConnControl;
import org.apache.hc.core5.http2.protocol.H2ResponseContent;
import org.apache.hc.core5.util.TextUtils;
Expand All @@ -55,13 +57,15 @@ public static HttpProcessorBuilder customServer(final String serverInfo) {
return HttpProcessorBuilder.create()
.addAll(
ResponseConformance.INSTANCE,
H2ResponseConformance.INSTANCE,
ResponseDate.INSTANCE,
new ResponseServer(!TextUtils.isBlank(serverInfo) ? serverInfo :
VersionInfo.getSoftwareInfo(SOFTWARE, "org.apache.hc.core5", H2Processors.class)),
H2ResponseContent.INSTANCE,
H2ResponseConnControl.INSTANCE)
.addAll(
H2RequestValidateHost.INSTANCE,
H2RequestConformance.INSTANCE,
RequestConformance.INSTANCE);
}

Expand All @@ -76,6 +80,9 @@ public static HttpProcessor server() {
public static HttpProcessorBuilder customClient(final String agentInfo) {
return HttpProcessorBuilder.create()
.addAll(
H2ResponseConformance.INSTANCE)
.addAll(
H2RequestConformance.INSTANCE,
H2RequestTargetHost.INSTANCE,
H2RequestContent.INSTANCE,
H2RequestConnControl.INSTANCE,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* ====================================================================
* 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.http2.protocol;

import java.io.IOException;

import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.Internal;
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.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.http.protocol.HttpContext;
import org.apache.hc.core5.util.Args;

/**
* This request interceptor is responsible for execution of the protocol conformance
* checks on incoming or outgoing HTTP/2 request messages.
*/
@Internal
@Contract(threading = ThreadingBehavior.IMMUTABLE)
public class H2RequestConformance implements HttpRequestInterceptor {

public static final H2RequestConformance INSTANCE = new H2RequestConformance();

private final String[] illegalHeaderNames;

@Internal
public H2RequestConformance(final String... illegalHeaderNames) {
super();
this.illegalHeaderNames = illegalHeaderNames;
}

public H2RequestConformance() {
this(
HttpHeaders.CONNECTION,
HttpHeaders.KEEP_ALIVE,
HttpHeaders.PROXY_CONNECTION,
HttpHeaders.TRANSFER_ENCODING,
HttpHeaders.HOST,
HttpHeaders.UPGRADE,
HttpHeaders.TE);
}

@Override
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
final Header header = request.getFirstHeader(headerName);
if (header != null) {
if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
final String value = header.getValue();
if (!"trailers".equalsIgnoreCase(value)) {
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, value);
}
} else {
throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
}
}
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* ====================================================================
* 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.http2.protocol;

import java.io.IOException;

import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.Internal;
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.HttpException;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.HttpResponseInterceptor;
import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.util.Args;

/**
* This response interceptor is responsible for making the protocol conformance checks
* of incoming or outgoing HTTP/2 response messages.
*/
@Internal
@Contract(threading = ThreadingBehavior.IMMUTABLE)
public class H2ResponseConformance implements HttpResponseInterceptor {

public static final H2ResponseConformance INSTANCE = new H2ResponseConformance();

private final String[] illegalHeaderNames;

@Internal
public H2ResponseConformance(final String... illegalHeaderNames) {
super();
this.illegalHeaderNames = illegalHeaderNames;
}

public H2ResponseConformance() {
this(
HttpHeaders.CONNECTION,
HttpHeaders.KEEP_ALIVE,
HttpHeaders.TRANSFER_ENCODING,
HttpHeaders.UPGRADE);
}

@Override
public void process(final HttpResponse response, final EntityDetails entity, final HttpContext context)
throws HttpException, IOException {
Args.notNull(response, "HTTP response");
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
final Header header = response.getFirstHeader(headerName);
if (header != null) {
throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,6 @@ void testConvertFromFieldsUpperCaseHeaderName() {
"Header name ':Path' is invalid (header name contains uppercase characters)");
}

@Test
void testConvertFromFieldsConnectionHeader() {
final List<Header> headers = Arrays.asList(
new BasicHeader(":method", "GET"),
new BasicHeader(":scheme", "http"),
new BasicHeader(":authority", "www.example.com"),
new BasicHeader(":path", "/"),
new BasicHeader("connection", "keep-alive"));

final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
Assertions.assertThrows(HttpException.class, () -> converter.convert(headers),
"Header 'connection: keep-alive' is illegal for HTTP/2 messages");
}

@Test
void testConvertFromFieldsPseudoHeaderSequence() {
final List<Header> headers = Arrays.asList(
Expand Down Expand Up @@ -346,76 +332,6 @@ void testConvertFromMessageConnectWithPath() {
"CONNECT request path must be null");
}

@Test
void testConvertFromMessageConnectionHeader() {
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
request.addHeader("Connection", "Keep-Alive");

final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
Assertions.assertThrows(HttpException.class, () -> converter.convert(request),
"Header 'Connection: Keep-Alive' is illegal for HTTP/2 messages");
}

@Test
void testConvertFromFieldsKeepAliveHeader() {
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
request.addHeader("Keep-Alive", "timeout=5, max=1000");

final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
Assertions.assertThrows(HttpException.class, () -> converter.convert(request),
"Header 'Keep-Alive: timeout=5, max=1000' is illegal for HTTP/2 messages");
}

@Test
void testConvertFromFieldsProxyConnectionHeader() {
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
request.addHeader("Proxy-Connection", "keep-alive");

final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
Assertions.assertThrows(HttpException.class, () -> converter.convert(request),
"Header 'Proxy-Connection: Keep-Alive' is illegal for HTTP/2 messages");
}

@Test
void testConvertFromFieldsTransferEncodingHeader() {
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
request.addHeader("Transfer-Encoding", "gzip");

final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
Assertions.assertThrows(HttpException.class, () -> converter.convert(request),
"Header 'Transfer-Encoding: gzip' is illegal for HTTP/2 messages");
}

@Test
void testConvertFromFieldsHostHeader() {
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
request.addHeader("Host", "host");

final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
Assertions.assertThrows(HttpException.class, () -> converter.convert(request),
"Header 'Host: host' is illegal for HTTP/2 messages");
}

@Test
void testConvertFromFieldsUpgradeHeader() {
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
request.addHeader("Upgrade", "example/1, foo/2");

final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
Assertions.assertThrows(HttpException.class, () -> converter.convert(request),
"Header 'Upgrade: example/1, foo/2' is illegal for HTTP/2 messages");
}

@Test
void testConvertFromFieldsTEHeader() {
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
request.addHeader("TE", "gzip");

final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
Assertions.assertThrows(HttpException.class, () -> converter.convert(request),
"Header 'TE: gzip' is illegal for HTTP/2 messages");
}

@Test
void testConvertFromFieldsTETrailerHeader() throws Exception {

Expand Down
Loading

0 comments on commit 549e239

Please sign in to comment.