Merge branch 'issue-2022' of https://github.com/Baqend/jetty.project into jetty-9.4.x-2022-FineGrainedComplianceModes

This commit is contained in:
Greg Wilkins 2017-12-15 16:19:14 +01:00
commit c6c5a3b890
6 changed files with 204 additions and 29 deletions

View File

@ -52,9 +52,11 @@ import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
import org.eclipse.jetty.client.util.FormContentProvider;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpParser;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.ClientConnectionFactory;
@ -147,6 +149,7 @@ public class HttpClient extends ContainerLifeCycle
private boolean removeIdleDestinations = false;
private boolean connectBlocking = false;
private String name = getClass().getSimpleName() + "@" + Integer.toHexString(hashCode());
private HttpCompliance httpCompliance = HttpCompliance.RFC7230;
/**
* Creates a {@link HttpClient} instance that can perform requests to non-TLS destinations only
@ -972,6 +975,25 @@ public class HttpClient extends ContainerLifeCycle
{
}
/**
* Gets the http compliance mode for parsing http responses.
* The default http compliance level is {@link HttpCompliance#RFC7230} which is the latest HTTP/1.1 specification
*/
public HttpCompliance getHttpCompliance()
{
return httpCompliance;
}
/**
* Sets the http compliance mode for parsing http responses.
* This affect how weak the {@link HttpParser} parses http responses and which http protocol level is supported
* @param httpCompliance The compliance level which is used to actually parse http responses
*/
public void setHttpCompliance(HttpCompliance httpCompliance)
{
this.httpCompliance = httpCompliance;
}
/**
* @return whether request events must be strictly ordered
* @see #setStrictEventOrdering(boolean)

View File

@ -26,6 +26,7 @@ import org.eclipse.jetty.client.HttpExchange;
import org.eclipse.jetty.client.HttpReceiver;
import org.eclipse.jetty.client.HttpResponse;
import org.eclipse.jetty.client.HttpResponseException;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpParser;
@ -38,13 +39,15 @@ import org.eclipse.jetty.util.CompletableCallback;
public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.ResponseHandler
{
private final HttpParser parser = new HttpParser(this);
private final HttpParser parser;
private ByteBuffer buffer;
private boolean shutdown;
public HttpReceiverOverHTTP(HttpChannelOverHTTP channel)
{
super(channel);
// TODO: Seems to be not the optimal way to get the compliance mode
parser = new HttpParser(this, -1, channel.getHttpDestination().getHttpClient().getHttpCompliance());
}
@Override

View File

@ -20,6 +20,8 @@ package org.eclipse.jetty.client.http;
import java.io.EOFException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
@ -33,32 +35,53 @@ import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.api.Connection;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.util.FutureResponseListener;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.io.ByteArrayEndPoint;
import org.eclipse.jetty.toolchain.test.TestTracker;
import org.eclipse.jetty.util.Promise;
import org.hamcrest.CoreMatchers;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@RunWith(Parameterized.class)
public class HttpReceiverOverHTTPTest
{
@Rule
public final TestTracker tracker = new TestTracker();
@Parameterized.Parameter(0)
public HttpCompliance compliance;
private HttpClient client;
private HttpDestinationOverHTTP destination;
private ByteArrayEndPoint endPoint;
private HttpConnectionOverHTTP connection;
@Parameterized.Parameters
public static Collection<Object[]> parameters() throws Exception
{
return Arrays.asList(
new Object[] { HttpCompliance.LEGACY },
new Object[] { HttpCompliance.WEAK },
new Object[] { HttpCompliance.RFC2616 },
new Object[] { HttpCompliance.RFC7230 }
);
}
@Before
public void init() throws Exception
{
client = new HttpClient();
client.setHttpCompliance(compliance);
client.start();
destination = new HttpDestinationOverHTTP(client, new Origin("http", "localhost", 8080));
destination.start();
@ -207,6 +230,37 @@ public class HttpReceiverOverHTTPTest
Assert.assertTrue(e.getCause() instanceof HttpResponseException);
}
}
@Test
public void test_Receive_Invalid_Response_With_Weak_Compliance() throws Exception
{
endPoint.addInput("" +
"HTTP/1.1 200 OK\r\n" +
"Content-length : 0\r\n" +
"\r\n");
HttpExchange exchange = newExchange();
FutureResponseListener listener = (FutureResponseListener)exchange.getResponseListeners().get(0);
connection.getHttpChannel().receive();
try {
Response response = listener.get(5, TimeUnit.SECONDS);
Assert.assertThat(compliance, Matchers.lessThanOrEqualTo(HttpCompliance.WEAK));
Assert.assertNotNull(response);
Assert.assertEquals(200, response.getStatus());
Assert.assertEquals("OK", response.getReason());
Assert.assertSame(HttpVersion.HTTP_1_1, response.getVersion());
HttpFields headers = response.getHeaders();
Assert.assertNotNull(headers);
Assert.assertEquals(1, headers.size());
Assert.assertEquals("0", headers.get(HttpHeader.CONTENT_LENGTH));
} catch (Exception e) {
Assert.assertThat(compliance, Matchers.greaterThan(HttpCompliance.WEAK));
Throwable cause = e.getCause();
Assert.assertThat(cause, CoreMatchers.instanceOf(HttpResponseException.class));
Assert.assertThat(cause.getMessage(), Matchers.containsString("HTTP protocol violation"));
}
}
@Test
public void test_FillInterested_RacingWith_BufferRelease() throws Exception

View File

@ -24,9 +24,11 @@ package org.eclipse.jetty.http;
* <dl>
* <dt>RFC7230</dt><dd>(default) Compliance with RFC7230</dd>
* <dt>RFC2616</dt><dd>Wrapped/Continued headers and HTTP/0.9 supported</dd>
* <dt>LEGACY</dt><dd>(aka STRICT) Adherence to Servlet Specification requirement for
* <dt>WEAK</dt><dd>Wrapped/Continued headers, HTTP/0.9 supported and make the parser more acceptable against miss
* formatted requests/responses</dd>
* <dt>LEGACY</dt><dd>(aka STRICT) Adherence to Servlet Specification requirement for
* exact case of header names, bypassing the header caches, which are case insensitive,
* otherwise equivalent to RFC2616</dd>
* otherwise equivalent to WEAK</dd>
* </dl>
*/
public enum HttpCompliance { LEGACY, RFC2616, RFC7230 }
public enum HttpCompliance { LEGACY, WEAK, RFC2616, RFC7230 }

View File

@ -36,6 +36,7 @@ import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import static org.eclipse.jetty.http.HttpCompliance.LEGACY;
import static org.eclipse.jetty.http.HttpCompliance.WEAK;
import static org.eclipse.jetty.http.HttpCompliance.RFC2616;
import static org.eclipse.jetty.http.HttpCompliance.RFC7230;
import static org.eclipse.jetty.http.HttpTokens.CARRIAGE_RETURN;
@ -82,6 +83,7 @@ import static org.eclipse.jetty.http.HttpTokens.TAB;
* <dl>
* <dt>RFC7230</dt><dd>(default) Compliance with RFC7230</dd>
* <dt>RFC2616</dt><dd>Wrapped headers and HTTP/0.9 supported</dd>
* <dt>WEAK</dt><dd>Wrapped headers, HTTP/0.9 supported and a weaker parsing behaviour</dd>
* <dt>LEGACY</dt><dd>(aka STRICT) Adherence to Servlet Specification requirement for
* exact case of header names, bypassing the header caches, which are case insensitive,
* otherwise equivalent to RFC2616</dd>
@ -116,6 +118,7 @@ public class HttpParser
IN_NAME,
VALUE,
IN_VALUE,
OWS_AFTER_NAME,
}
// States
@ -333,7 +336,7 @@ public class HttpParser
/* ------------------------------------------------------------------------------- */
protected String caseInsensitiveHeader(String orig, String normative)
{
return (_compliance!=LEGACY || orig.equals(normative) || complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 case sensitive header: "+orig))
return (_compliance!=LEGACY || orig.equals(normative) || complianceViolation(WEAK,"https://tools.ietf.org/html/rfc2616#section-4.2 case sensitive header: "+orig))
?normative:orig;
}
@ -658,6 +661,7 @@ public class HttpParser
// Legacy correctly allows case sensitive header;
break;
case WEAK:
case RFC2616:
case RFC7230:
if (!method.asString().equals(_methodString) && _complianceHandler!=null)
@ -1244,6 +1248,22 @@ public class HttpParser
break;
case IN_NAME:
if (b>HttpTokens.SPACE && b!=HttpTokens.COLON)
{
if (_header!=null)
{
setString(_header.asString());
_header=null;
_headerString=null;
}
_string.append((char)b);
_length=_string.length();
break;
}
// Fallthrough
case OWS_AFTER_NAME:
if (b==HttpTokens.COLON)
{
if (_headerString==null)
@ -1256,23 +1276,8 @@ public class HttpParser
setState(FieldState.VALUE);
break;
}
if (b>HttpTokens.SPACE)
{
if (_header!=null)
{
setString(_header.asString());
_header=null;
_headerString=null;
}
_string.append((char)b);
if (b>HttpTokens.SPACE)
_length=_string.length();
break;
}
if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2 No colon"))
if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 No colon"))
{
if (_headerString==null)
{
@ -1287,6 +1292,13 @@ public class HttpParser
break;
}
//Ignore trailing whitespaces
if (b==HttpTokens.SPACE && !complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 Invalid token in header name"))
{
setState(FieldState.OWS_AFTER_NAME);
break;
}
throw new IllegalCharacterException(_state,b,buffer);
case VALUE:

View File

@ -21,6 +21,7 @@ package org.eclipse.jetty.http;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.eclipse.jetty.http.HttpParser.State;
@ -350,7 +351,7 @@ public class HttpParserTest
}
@Test
public void testNoColonLegacy() throws Exception
public void testNoColonWeak() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
@ -360,7 +361,7 @@ public class HttpParserTest
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler,HttpCompliance.LEGACY);
HttpParser parser = new HttpParser(handler,HttpCompliance.WEAK);
parseAll(parser, buffer);
Assert.assertTrue(_headerCompleted);
@ -377,7 +378,87 @@ public class HttpParserTest
Assert.assertEquals(2, _headers);
Assert.assertThat(_complianceViolation, Matchers.containsString("No colon"));
}
@Test
public void testNoColonWithWhitespaceWeak() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"Name \r\n" +
"Other: value\r\n" +
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler,HttpCompliance.WEAK);
parseAll(parser, buffer);
Assert.assertTrue(_headerCompleted);
Assert.assertTrue(_messageCompleted);
Assert.assertEquals("GET", _methodOrVersion);
Assert.assertEquals("/", _uriOrStatus);
Assert.assertEquals("HTTP/1.0", _versionOrReason);
Assert.assertEquals("Host", _hdr[0]);
Assert.assertEquals("localhost", _val[0]);
Assert.assertEquals("Name", _hdr[1]);
Assert.assertEquals("", _val[1]);
Assert.assertEquals("Other", _hdr[2]);
Assert.assertEquals("value", _val[2]);
Assert.assertEquals(2, _headers);
Assert.assertThat(_complianceViolation, Matchers.containsString("No colon"));
}
@Test
public void testTrailingSpacesInHeaderNameInWeakMode() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"HTTP/1.1 204 No Content\r\n" +
"Access-Control-Allow-Headers : Origin\r\n" +
"Other: value\r\n" +
"\r\n");
HttpParser.ResponseHandler handler = new Handler();
HttpParser parser = new HttpParser(handler, -1, HttpCompliance.WEAK);
parseAll(parser, buffer);
Assert.assertTrue(_headerCompleted);
Assert.assertTrue(_messageCompleted);
Assert.assertEquals("HTTP/1.1", _methodOrVersion);
Assert.assertEquals("204", _uriOrStatus);
Assert.assertEquals("No Content", _versionOrReason);
Assert.assertEquals(null, _content);
Assert.assertEquals(1, _headers);
System.out.println(Arrays.asList(_hdr));
System.out.println(Arrays.asList(_val));
Assert.assertEquals("Access-Control-Allow-Headers", _hdr[0]);
Assert.assertEquals("Origin", _val[0]);
Assert.assertEquals("Other", _hdr[1]);
Assert.assertEquals("value", _val[1]);
Assert.assertThat(_complianceViolation, Matchers.containsString("Invalid token in header name"));
}
@Test
public void testTrailingSpacesInHeaderNameNoWeak() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"HTTP/1.1 204 No Content\r\n" +
"Access-Control-Allow-Headers : Origin\r\n" +
"Other: value\r\n" +
"\r\n");
HttpParser.ResponseHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);
parseAll(parser, buffer);
Assert.assertEquals("HTTP/1.1", _methodOrVersion);
Assert.assertEquals("204", _uriOrStatus);
Assert.assertEquals("No Content", _versionOrReason);
Assert.assertThat(_bad, Matchers.containsString("Illegal character 0x20"));
}
@Test
public void testNoColon7230() throws Exception
{
@ -1366,10 +1447,10 @@ public class HttpParserTest
@Test
public void testResponseReasonIso8859_1() throws Exception
{
{
ByteBuffer buffer = BufferUtil.toBuffer(
"HTTP/1.1 302 déplacé temporairement\r\n"
+ "Content-Length: 0\r\n"
+ "Content-Length: 0\r\n"
+ "\r\n",StandardCharsets.ISO_8859_1);
HttpParser.ResponseHandler handler = new Handler();
@ -2147,8 +2228,9 @@ public class HttpParserTest
_methodOrVersion = version.asString();
_uriOrStatus = Integer.toString(status);
_versionOrReason = reason;
_hdr = new String[9];
_val = new String[9];
_headers = -1;
_hdr = new String[10];
_val = new String[10];
_messageCompleted = false;
_headerCompleted = false;
return false;