Skip to content

Commit 6741e3c

Browse files
yinwoodsok2c
yinwoods
authored and
ok2c
committed
HTTPCORE-692 add new rules for H2 header check as rfc7540 section 8.1.2.2 and 8.1.2.3 defined
1 parent 5b5a6e6 commit 6741e3c

File tree

5 files changed

+139
-6
lines changed

5 files changed

+139
-6
lines changed

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/DefaultH2RequestConverter.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,12 @@ public HttpRequest convert(final List<Header> headers) throws HttpException {
106106
throw new ProtocolException("Unsupported request header '%s'", name);
107107
}
108108
} else {
109-
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION)) {
109+
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION) || name.equalsIgnoreCase(HttpHeaders.KEEP_ALIVE)
110+
|| name.equalsIgnoreCase(HttpHeaders.PROXY_CONNECTION) || name.equalsIgnoreCase(HttpHeaders.TRANSFER_ENCODING)
111+
|| name.equalsIgnoreCase(HttpHeaders.HOST) || name.equalsIgnoreCase(HttpHeaders.UPGRADE)) {
112+
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
113+
}
114+
if (name.equalsIgnoreCase(HttpHeaders.TE) && !value.equalsIgnoreCase("trailers")) {
110115
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
111116
}
112117
messageHeaders.add(header);
@@ -189,8 +194,13 @@ public List<Header> convert(final HttpRequest message) throws HttpException {
189194
if (name.startsWith(":")) {
190195
throw new ProtocolException("Header name '%s' is invalid", name);
191196
}
192-
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION)) {
193-
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", name, value);
197+
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION) || name.equalsIgnoreCase(HttpHeaders.KEEP_ALIVE)
198+
|| name.equalsIgnoreCase(HttpHeaders.PROXY_CONNECTION) || name.equalsIgnoreCase(HttpHeaders.TRANSFER_ENCODING)
199+
|| name.equalsIgnoreCase(HttpHeaders.HOST) || name.equalsIgnoreCase(HttpHeaders.UPGRADE)) {
200+
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
201+
}
202+
if (name.equalsIgnoreCase(HttpHeaders.TE) && !value.equalsIgnoreCase("trailers")) {
203+
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
194204
}
195205
headers.add(new BasicHeader(TextUtils.toLowerCase(name), value));
196206
}

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/DefaultH2ResponseConverter.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ public HttpResponse convert(final List<Header> headers) throws HttpException {
8282
throw new ProtocolException("Unsupported response header '%s'", name);
8383
}
8484
} else {
85-
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION)) {
85+
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION) || name.equalsIgnoreCase(HttpHeaders.KEEP_ALIVE)
86+
|| name.equalsIgnoreCase(HttpHeaders.TRANSFER_ENCODING) || name.equalsIgnoreCase(HttpHeaders.UPGRADE)) {
8687
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
8788
}
8889
messageHeaders.add(header);
@@ -123,8 +124,9 @@ public List<Header> convert(final HttpResponse message) throws HttpException {
123124
if (name.startsWith(":")) {
124125
throw new ProtocolException("Header name '%s' is invalid", name);
125126
}
126-
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION)) {
127-
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", name, value);
127+
if (name.equalsIgnoreCase(HttpHeaders.CONNECTION) || name.equalsIgnoreCase(HttpHeaders.KEEP_ALIVE)
128+
|| name.equalsIgnoreCase(HttpHeaders.TRANSFER_ENCODING) || name.equalsIgnoreCase(HttpHeaders.UPGRADE)) {
129+
throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", header.getName(), header.getValue());
128130
}
129131
headers.add(new BasicHeader(TextUtils.toLowerCase(name), value));
130132
}

‎httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/TestDefaultH2RequestConverter.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,89 @@ public void testConvertFromMessageConnectionHeader() throws Exception {
358358
HttpException.class, () -> converter.convert(request));
359359
}
360360

361+
@Test
362+
public void testConvertFromFieldsKeepAliveHeader() throws Exception {
363+
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
364+
request.addHeader("Keep-Alive", "timeout=5, max=1000");
365+
366+
final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
367+
Assert.assertThrows("Header 'Keep-Alive: timeout=5, max=1000' is illegal for HTTP/2 messages",
368+
HttpException.class, () -> converter.convert(request));
369+
}
370+
371+
@Test
372+
public void testConvertFromFieldsProxyConnectionHeader() throws Exception {
373+
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
374+
request.addHeader("Proxy-Connection", "keep-alive");
375+
376+
final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
377+
Assert.assertThrows("Header 'Proxy-Connection: Keep-Alive' is illegal for HTTP/2 messages",
378+
HttpException.class, () -> converter.convert(request));
379+
}
380+
381+
@Test
382+
public void testConvertFromFieldsTransferEncodingHeader() throws Exception {
383+
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
384+
request.addHeader("Transfer-Encoding", "gzip");
385+
386+
final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
387+
Assert.assertThrows("Header 'Transfer-Encoding: gzip' is illegal for HTTP/2 messages",
388+
HttpException.class, () -> converter.convert(request));
389+
}
390+
391+
@Test
392+
public void testConvertFromFieldsHostHeader() throws Exception {
393+
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
394+
request.addHeader("Host", "host");
395+
396+
final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
397+
Assert.assertThrows("Header 'Host: host' is illegal for HTTP/2 messages",
398+
HttpException.class, () -> converter.convert(request));
399+
}
400+
401+
@Test
402+
public void testConvertFromFieldsUpgradeHeader() throws Exception {
403+
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
404+
request.addHeader("Upgrade", "example/1, foo/2");
405+
406+
final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
407+
Assert.assertThrows("Header 'Upgrade: example/1, foo/2' is illegal for HTTP/2 messages",
408+
HttpException.class, () -> converter.convert(request));
409+
}
410+
411+
@Test
412+
public void testConvertFromFieldsTEHeader() throws Exception {
413+
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");
414+
request.addHeader("TE", "gzip");
415+
416+
final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
417+
Assert.assertThrows("Header 'TE: gzip' is illegal for HTTP/2 messages",
418+
HttpException.class, () -> converter.convert(request));
419+
}
420+
421+
@Test
422+
public void testConvertFromFieldsTETrailerHeader() throws Exception {
423+
424+
final List<Header> headers = Arrays.asList(
425+
new BasicHeader(":method", "GET"),
426+
new BasicHeader(":scheme", "http"),
427+
new BasicHeader(":authority", "www.example.com"),
428+
new BasicHeader(":path", "/"),
429+
new BasicHeader("te", "trailers"));
430+
431+
final DefaultH2RequestConverter converter = new DefaultH2RequestConverter();
432+
final HttpRequest request = converter.convert(headers);
433+
Assert.assertNotNull(request);
434+
Assert.assertEquals("GET", request.getMethod());
435+
Assert.assertEquals("http", request.getScheme());
436+
Assert.assertEquals(new URIAuthority("www.example.com"), request.getAuthority());
437+
Assert.assertEquals("/", request.getPath());
438+
final Header[] allHeaders = request.getHeaders();
439+
Assert.assertEquals(1, allHeaders.length);
440+
Assert.assertEquals("te", allHeaders[0].getName());
441+
Assert.assertEquals("trailers", allHeaders[0].getValue());
442+
}
443+
361444
@Test
362445
public void testConvertFromMessageInvalidHeader() throws Exception {
363446
final HttpRequest request = new BasicHttpRequest("GET", new HttpHost("host"), "/");

‎httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/TestDefaultH2ResponseConverter.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,42 @@ public void testConvertFromFieldsConnectionHeader() throws Exception {
9595
HttpException.class, () -> converter.convert(headers));
9696
}
9797

98+
@Test
99+
public void testConvertFromFieldsKeepAliveHeader() throws Exception {
100+
final List<Header> headers = Arrays.asList(
101+
new BasicHeader(":status", "200"),
102+
new BasicHeader("location", "http://www.example.com/"),
103+
new BasicHeader("keep-alive", "timeout=5, max=1000"));
104+
105+
final DefaultH2ResponseConverter converter = new DefaultH2ResponseConverter();
106+
Assert.assertThrows("Header 'keep-alive: timeout=5, max=1000' is illegal for HTTP/2 messages",
107+
HttpException.class, () -> converter.convert(headers));
108+
}
109+
110+
@Test
111+
public void testConvertFromFieldsTransferEncodingHeader() throws Exception {
112+
final List<Header> headers = Arrays.asList(
113+
new BasicHeader(":status", "200"),
114+
new BasicHeader("location", "http://www.example.com/"),
115+
new BasicHeader("transfer-encoding", "gzip"));
116+
117+
final DefaultH2ResponseConverter converter = new DefaultH2ResponseConverter();
118+
Assert.assertThrows("Header 'transfer-encoding: gzip' is illegal for HTTP/2 messages",
119+
HttpException.class, () -> converter.convert(headers));
120+
}
121+
122+
@Test
123+
public void testConvertFromFieldsUpgradeHeader() throws Exception {
124+
final List<Header> headers = Arrays.asList(
125+
new BasicHeader(":status", "200"),
126+
new BasicHeader("location", "http://www.example.com/"),
127+
new BasicHeader("upgrade", "example/1, foo/2"));
128+
129+
final DefaultH2ResponseConverter converter = new DefaultH2ResponseConverter();
130+
Assert.assertThrows("Header 'upgrade: example/1, foo/2' is illegal for HTTP/2 messages",
131+
HttpException.class, () -> converter.convert(headers));
132+
}
133+
98134
@Test
99135
public void testConvertFromFieldsMissingStatus() throws Exception {
100136
final List<Header> headers = Arrays.asList(

‎httpcore5/src/main/java/org/apache/hc/core5/http/HttpHeaders.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ private HttpHeaders() {
167167

168168
public static final String PROXY_AUTHORIZATION = "Proxy-Authorization";
169169

170+
public static final String PROXY_CONNECTION = "Proxy-Connection";
171+
170172
public static final String RANGE = "Range";
171173

172174
public static final String REFERER = "Referer";

0 commit comments

Comments
 (0)