From 4679565fd5b33fc009a9193bc5f65c6f67436e75 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 22 Oct 2018 13:10:05 -0500 Subject: [PATCH 1/2] Issue #3011 - Moving HttpCompliance to HttpConfiguration Signed-off-by: Joakim Erdfelt --- .../jetty/server/HttpConfiguration.java | 12 ++++++++++++ .../jetty/server/HttpConnectionFactory.java | 19 ++----------------- .../jetty/server/HttpConnectionTest.java | 10 +++++----- .../servlet/ComplianceViolations2616Test.java | 3 ++- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java index a6a19ff5d477..7af7ba11e6e8 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java @@ -24,6 +24,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.http.CookieCompliance; +import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.util.Jetty; @@ -71,6 +72,7 @@ public class HttpConfiguration private boolean _useDirectByteBuffers = false; private long _minRequestDataRate; private long _minResponseDataRate; + private HttpCompliance _httpCompliance = HttpCompliance.RFC7230; private CookieCompliance _cookieCompliance = CookieCompliance.RFC6265; private MultiPartFormDataCompliance _multiPartCompliance = MultiPartFormDataCompliance.RFC7578; private boolean _notifyRemoteAsyncErrors = true; @@ -547,6 +549,16 @@ public void setMinResponseDataRate(long bytesPerSecond) _minResponseDataRate = bytesPerSecond; } + public HttpCompliance getHttpCompliance() + { + return _httpCompliance; + } + + public void setHttpCompliance(HttpCompliance _httpCompliance) + { + this._httpCompliance = _httpCompliance; + } + public CookieCompliance getCookieCompliance() { return _cookieCompliance; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java index 6459b0394d1a..708a51556416 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java @@ -32,7 +32,6 @@ public class HttpConnectionFactory extends AbstractConnectionFactory implements HttpConfiguration.ConnectionFactory { private final HttpConfiguration _config; - private HttpCompliance _httpCompliance; private boolean _recordHttpComplianceViolations = false; public HttpConnectionFactory() @@ -41,15 +40,9 @@ public HttpConnectionFactory() } public HttpConnectionFactory(@Name("config") HttpConfiguration config) - { - this(config,null); - } - - public HttpConnectionFactory(@Name("config") HttpConfiguration config, @Name("compliance") HttpCompliance compliance) { super(HttpVersion.HTTP_1_1.asString()); _config=config; - _httpCompliance=compliance==null?HttpCompliance.RFC7230:compliance; if (config==null) throw new IllegalArgumentException("Null HttpConfiguration"); addBean(_config); @@ -63,7 +56,7 @@ public HttpConfiguration getHttpConfiguration() public HttpCompliance getHttpCompliance() { - return _httpCompliance; + return _config.getHttpCompliance(); } public boolean isRecordHttpComplianceViolations() @@ -71,18 +64,10 @@ public boolean isRecordHttpComplianceViolations() return _recordHttpComplianceViolations; } - /** - * @param httpCompliance String value of {@link HttpCompliance} - */ - public void setHttpCompliance(HttpCompliance httpCompliance) - { - _httpCompliance = httpCompliance; - } - @Override public Connection newConnection(Connector connector, EndPoint endPoint) { - HttpConnection conn = new HttpConnection(_config, connector, endPoint, _httpCompliance,isRecordHttpComplianceViolations()); + HttpConnection conn = new HttpConnection(_config, connector, endPoint, getHttpCompliance(), isRecordHttpComplianceViolations()); return configure(conn, connector, endPoint); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java index 811b22a80938..d29567e3c7c6 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java @@ -148,12 +148,12 @@ public void testFragmentedChunk() throws Exception @Test public void testHttp09_NoVersion() throws Exception { - connector.getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616); String request = "GET / HTTP/0.9\r\n\r\n"; String response = connector.getResponse(request); assertThat(response, containsString("400 Bad Version")); - connector.getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC7230); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC7230); request = "GET / HTTP/0.9\r\n\r\n"; response = connector.getResponse(request); assertThat(response, containsString("400 Bad Version")); @@ -165,7 +165,7 @@ public void testHttp09_NoVersion() throws Exception @Test public void testHttp09_NoHeaders() throws Exception { - connector.getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616); // header looking like another request is ignored String request = "GET /one\r\nGET :/two\r\n\r\n"; String response = BufferUtil.toString(connector.executeRequest(request).waitForOutput(10,TimeUnit.SECONDS)); @@ -179,7 +179,7 @@ public void testHttp09_NoHeaders() throws Exception @Test public void testHttp09_MultipleRequests() throws Exception { - connector.getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616); // Verify that pipelining does not work with HTTP/0.9. String requests = "GET /?id=123\r\n\r\nGET /?id=456\r\n\r\n"; @@ -407,7 +407,7 @@ public void testEncodedBadDotDotPath() throws Exception @Test public void test_0_9() throws Exception { - connector.getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616_LEGACY); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616_LEGACY); LocalEndPoint endp = connector.executeRequest("GET /R1\n"); endp.waitUntilClosed(); String response=BufferUtil.toString(endp.takeOutput()); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java index f9b8d4303103..e3ee9ecc916b 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java @@ -109,8 +109,9 @@ public static void startServer() throws Exception HttpConfiguration config = new HttpConfiguration(); config.setSendServerVersion(false); + config.setHttpCompliance(HttpCompliance.RFC2616_LEGACY); - HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory(config, HttpCompliance.RFC2616_LEGACY); + HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory(config); httpConnectionFactory.setRecordHttpComplianceViolations(true); connector = new LocalConnector(server, null, null, null, -1, httpConnectionFactory); From fbd9fc1da7f0522e00a71a02bc5e96488ddb7363 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 22 Oct 2018 16:12:11 -0500 Subject: [PATCH 2/2] Issue #3011 - Removing HttpConnectionFactory.getHttpCompliance() + Can get HttpCompliance from HttpConfiguration now + Signature change to HttpConnection to avoid duplicate arguments on constructor. Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/client/ssl/SslBytesServerTest.java | 2 +- .../org/eclipse/jetty/http2/server/HTTP2CServerTest.java | 2 +- .../java/org/eclipse/jetty/server/HttpConnection.java | 4 ++-- .../org/eclipse/jetty/server/HttpConnectionFactory.java | 8 +------- .../java/org/eclipse/jetty/server/ExtendedServerTest.java | 3 +-- .../java/org/eclipse/jetty/server/GracefulStopTest.java | 2 +- .../jetty/server/SlowClientWithPipelinedRequestTest.java | 2 +- 7 files changed, 8 insertions(+), 15 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java index 1fc702350435..7aeb436028d3 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java @@ -128,7 +128,7 @@ public void init() throws Exception @Override public Connection newConnection(Connector connector, EndPoint endPoint) { - return configure(new HttpConnection(getHttpConfiguration(), connector, endPoint,getHttpCompliance(),isRecordHttpComplianceViolations()) + return configure(new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations()) { @Override protected HttpParser newHttpParser(HttpCompliance compliance) diff --git a/jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/HTTP2CServerTest.java b/jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/HTTP2CServerTest.java index 649ea7048cbb..0fc087e71965 100644 --- a/jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/HTTP2CServerTest.java +++ b/jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/HTTP2CServerTest.java @@ -300,7 +300,7 @@ public void testHTTP_2_0_DirectWithoutH2C() throws Exception @Override public Connection newConnection(Connector connector, EndPoint endPoint) { - HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint,getHttpCompliance(),isRecordHttpComplianceViolations()) + HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations()) { @Override public void onFillable() diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java index 5ce26824a6e9..8f712d32ed02 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java @@ -93,7 +93,7 @@ protected static HttpConnection setCurrentConnection(HttpConnection connection) return last; } - public HttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint, HttpCompliance compliance, boolean recordComplianceViolations) + public HttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint, boolean recordComplianceViolations) { super(endPoint, connector.getExecutor()); _config = config; @@ -102,7 +102,7 @@ public HttpConnection(HttpConfiguration config, Connector connector, EndPoint en _generator = newHttpGenerator(); _channel = newHttpChannel(); _input = _channel.getRequest().getHttpInput(); - _parser = newHttpParser(compliance); + _parser = newHttpParser(config.getHttpCompliance()); _recordHttpComplianceViolations = recordComplianceViolations; if (LOG.isDebugEnabled()) LOG.debug("New HTTP Connection {}", this); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java index 708a51556416..6758ce7c435f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java @@ -18,7 +18,6 @@ package org.eclipse.jetty.server; -import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.EndPoint; @@ -54,11 +53,6 @@ public HttpConfiguration getHttpConfiguration() return _config; } - public HttpCompliance getHttpCompliance() - { - return _config.getHttpCompliance(); - } - public boolean isRecordHttpComplianceViolations() { return _recordHttpComplianceViolations; @@ -67,7 +61,7 @@ public boolean isRecordHttpComplianceViolations() @Override public Connection newConnection(Connector connector, EndPoint endPoint) { - HttpConnection conn = new HttpConnection(_config, connector, endPoint, getHttpCompliance(), isRecordHttpComplianceViolations()); + HttpConnection conn = new HttpConnection(_config, connector, endPoint, isRecordHttpComplianceViolations()); return configure(conn, connector, endPoint); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java index 8b0c765638eb..ad7081a5167d 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java @@ -33,7 +33,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.ChannelEndPoint; import org.eclipse.jetty.io.Connection; @@ -104,7 +103,7 @@ private static class ExtendedHttpConnection extends HttpConnection { public ExtendedHttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint) { - super(config,connector,endPoint,HttpCompliance.RFC7230_LEGACY,false); + super(config,connector,endPoint, false); } @Override diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java index 2cb23ede8a40..4b5463d82ce7 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java @@ -281,7 +281,7 @@ public void testSlowClose(long stopTimeout, long closeWait, Matcher stopTi public Connection newConnection(Connector con, EndPoint endPoint) { // Slow closing connection - HttpConnection conn = new HttpConnection(getHttpConfiguration(), con, endPoint, getHttpCompliance(), isRecordHttpComplianceViolations()) + HttpConnection conn = new HttpConnection(getHttpConfiguration(), con, endPoint, isRecordHttpComplianceViolations()) { @Override public void close() diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java index 3cd93a4b4c38..5542a0d17399 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java @@ -57,7 +57,7 @@ public void startServer(Handler handler) throws Exception @Override public Connection newConnection(Connector connector, EndPoint endPoint) { - return configure(new HttpConnection(getHttpConfiguration(),connector,endPoint,getHttpCompliance(),isRecordHttpComplianceViolations()) + return configure(new HttpConnection(getHttpConfiguration(),connector,endPoint, isRecordHttpComplianceViolations()) { @Override public void onFillable()