Skip to content

Commit

Permalink
Issue #8716 - Updates from review
Browse files Browse the repository at this point in the history
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
joakime committed Oct 25, 2022
1 parent 615837a commit d183957
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ public HostPortHttpField(String authority)
protected HostPortHttpField(HttpHeader header, String name, String authority)
{
super(header, name, authority);

// only perform http field value split if a comma is present.
// this should skip 99% of authorities.
// It should catch bad Host fields with multiple values
// and allow things (like IDN's with a comma)
if (authority.contains(","))
{
QuotedCSV csv = new QuotedCSV(authority);
// TODO: should we allow multiple entries if all but 1 entry is blank/null?
if (csv.size() > 1)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad Authority");
}

try
{
_hostPort = new HostPort(authority);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//
// ========================================================================
// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//

package org.eclipse.jetty.http;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class HostPortHttpFieldTest
{
@Test
public void testSimpleAuthority()
{
HostPortHttpField hostPortHttpField = new HostPortHttpField("example.com");
assertThat(hostPortHttpField.getHost(), is("example.com"));
}

@Test
public void testMultiValueAuthorityField()
{
assertThrows(BadMessageException.class, () -> new HostPortHttpField("example.com, a.com, b.com, c.com"));
}

@ParameterizedTest
@ValueSource(strings = {
"क्\u200Dष", // Devanagari - [ka क] [virāma ्] [ZWJ] [ṣa ष]
"ರ\u200D್ಕ", // Kannada - [ra ರ‍] [ZWJ] [virāma ್] [ka ಕ]
"ශ්\u200Dර", // Sinhala - [śa ශ] [virāma ්] [ZWJ] [ra ර]
"ണ്\u200D" // Malayam - [Na ണ] [virāma ്] [ZWJ]
})
public void testIDNWithZeroWidthJoiner(String idn)
{
HostPortHttpField hostPortHttpField = new HostPortHttpField(idn);
assertThat(hostPortHttpField.getHost(), is(idn));
}
}
24 changes: 13 additions & 11 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand Down Expand Up @@ -2042,25 +2043,26 @@ public void testHostPort()

@ParameterizedTest
@ValueSource(strings = {
"Host: whatever.com:xxxx\r\n",
"Host: myhost:testBadPort\r\n",
"Host: a b c d\r\n",
"Host: hosta, hostb, hostc\r\n",
"Host: hosta,hostb,hostc\r\n",
"Host: hosta\r\nHost: hostb\r\nHost: hostc\r\n"
"Host: whatever.com:xxxx",
"Host: myhost:testBadPort",
"Host: a b c d",
"Host: a\to\tz",
"Host: hosta, hostb, hostc",
"Host: hosta,hostb,hostc",
"Host: hosta\nHost: hostb\nHost: hostc" // multi-line
})
public void testBadHost(String hostline)
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.1\r\n" +
hostline +
"Connection: close\r\n" +
"\r\n");
"GET / HTTP/1.1\n" +
hostline + "\n" +
"Connection: close\n" +
"\n");

HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);
parser.parseNext(buffer);
assertThat(_bad, containsString("Bad Host"));
assertThat(_bad, startsWith("Bad"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -787,12 +787,13 @@ public void testEncodedNotParams() throws Exception

@ParameterizedTest
@ValueSource(strings = {
"Host: whatever.com:xxxx\r\n",
"Host: myhost:testBadPort\r\n",
"Host: a b c d\r\n",
"Host: hosta, hostb, hostc\r\n",
"Host: hosta,hostb,hostc\r\n",
"Host: hosta\r\nHost: hostb\r\nHost: hostc\r\n"
"Host: whatever.com:xxxx",
"Host: myhost:testBadPort",
"Host: a b c d",
"Host: a\to\tz",
"Host: hosta, hostb, hostc",
"Host: hosta,hostb,hostc",
"Host: hosta\nHost: hostb\nHost: hostc" // multi-line
})
public void testInvalidHostHeader(String hostline) throws Exception
{
Expand All @@ -805,7 +806,7 @@ public void testInvalidHostHeader(String hostline) throws Exception

// Request with illegal Host header
String request = "GET / HTTP/1.1\n" +
hostline +
hostline + "\n" +
"Content-Type: text/html;charset=utf8\n" +
"Connection: close\n" +
"\n";
Expand Down
8 changes: 2 additions & 6 deletions jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,8 @@ else if (authority.charAt(0) == '[')
}
else
{
if (!isValidAuthority(authority))
{
if (!isAuthorityValid(authority))
throw new IllegalArgumentException("Bad Authority");
}
_host = authority;
_port = 0;
}
Expand All @@ -106,15 +104,13 @@ else if (authority.charAt(0) == '[')
* @param authority the authority to test
* @return true if the authority passes as valid
*/
private boolean isValidAuthority(String authority)
private boolean isAuthorityValid(String authority)
{
if (authority == null)
return false;
for (int i = 0; i < authority.length(); i++)
{
int codepoint = authority.codePointAt(i);
if (codepoint == ',')
return false;
if (Character.isISOControl(codepoint))
return false;
if (Character.isWhitespace(codepoint))
Expand Down

0 comments on commit d183957

Please sign in to comment.