From f8449868e654cf998bb60aed6842db9b9128e002 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 5 Nov 2019 18:40:27 +0100 Subject: [PATCH] Fixes #4258 - RateControl should be per-connection. Introduced RateControl.Factory to create instances of RateControl for each connection. Modified relevant XML files and added distribution test for h2. Signed-off-by: Simone Bordet --- .../jetty/http2/parser/RateControl.java | 14 +++++++ .../jetty/http2/parser/WindowRateControl.java | 16 ++++++++ .../src/main/config/etc/jetty-http2.xml | 10 ++--- .../src/main/config/etc/jetty-http2c.xml | 10 ++--- .../AbstractHTTP2ServerConnectionFactory.java | 38 ++++++++++++++++--- .../tests/distribution/DistributionTests.java | 22 +++++++++-- 6 files changed, 91 insertions(+), 19 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/RateControl.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/RateControl.java index be8f82a7cf2..76952e0152d 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/RateControl.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/RateControl.java @@ -36,4 +36,18 @@ public interface RateControl * @return true IFF the rate is within limits */ public boolean onEvent(Object event); + + /** + * Factory to create RateControl instances. + */ + public interface Factory + { + /** + * @return a new RateControl instance. + */ + public default RateControl newRateControl() + { + return NO_RATE_CONTROL; + } + } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowRateControl.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowRateControl.java index 4c00b01218b..fd1e83942ac 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowRateControl.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowRateControl.java @@ -67,4 +67,20 @@ public class WindowRateControl implements RateControl events.add(now + window); return size.incrementAndGet() <= maxEvents; } + + public static class Factory implements RateControl.Factory + { + private final int maxEventRate; + + public Factory(int maxEventRate) + { + this.maxEventRate = maxEventRate; + } + + @Override + public RateControl newRateControl() + { + return WindowRateControl.fromEventsPerSecond(maxEventRate); + } + } } diff --git a/jetty-http2/http2-server/src/main/config/etc/jetty-http2.xml b/jetty-http2/http2-server/src/main/config/etc/jetty-http2.xml index 43a59e88c51..398ac69caad 100644 --- a/jetty-http2/http2-server/src/main/config/etc/jetty-http2.xml +++ b/jetty-http2/http2-server/src/main/config/etc/jetty-http2.xml @@ -1,6 +1,6 @@ - + + - @@ -10,10 +10,10 @@ - - + + - + diff --git a/jetty-http2/http2-server/src/main/config/etc/jetty-http2c.xml b/jetty-http2/http2-server/src/main/config/etc/jetty-http2c.xml index a9e4380c0aa..be655550cc8 100644 --- a/jetty-http2/http2-server/src/main/config/etc/jetty-http2c.xml +++ b/jetty-http2/http2-server/src/main/config/etc/jetty-http2c.xml @@ -1,6 +1,6 @@ - + + - @@ -9,10 +9,10 @@ - - + + - + diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java index 7b5319efbc5..f6b7dc70c90 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.http2.server; import java.io.IOException; -import java.time.Duration; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -61,7 +60,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne private int maxHeaderBlockFragment = 0; private int maxFrameLength = Frame.DEFAULT_MAX_LENGTH; private int maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS; - private RateControl rateControl = new WindowRateControl(20, Duration.ofSeconds(1)); + private RateControl.Factory rateControlFactory = new WindowRateControl.Factory(20); private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F); private long streamIdleTimeout; @@ -182,14 +181,43 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne this.maxSettingsKeys = maxSettingsKeys; } + /** + * @return null + * @deprecated use {@link #getRateControlFactory()} instead + */ + @Deprecated public RateControl getRateControl() { - return rateControl; + return null; } + /** + * @param rateControl ignored + * @throws UnsupportedOperationException when invoked + * @deprecated use {@link #setRateControlFactory(RateControl.Factory)} instead + */ + @Deprecated public void setRateControl(RateControl rateControl) { - this.rateControl = rateControl; + throw new UnsupportedOperationException(); + } + + /** + * @return the factory that creates RateControl objects + */ + public RateControl.Factory getRateControlFactory() + { + return rateControlFactory; + } + + /** + *

Sets the factory that creates a per-connection RateControl object.

+ * + * @param rateControlFactory the factory that creates RateControl objects + */ + public void setRateControlFactory(RateControl.Factory rateControlFactory) + { + this.rateControlFactory = Objects.requireNonNull(rateControlFactory); } /** @@ -251,7 +279,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne session.setInitialSessionRecvWindow(getInitialSessionRecvWindow()); session.setWriteThreshold(getHttpConfiguration().getOutputBufferSize()); - ServerParser parser = newServerParser(connector, session, getRateControl()); + ServerParser parser = newServerParser(connector, session, getRateControlFactory().newRateControl()); parser.setMaxFrameLength(getMaxFrameLength()); parser.setMaxSettingsKeys(getMaxSettingsKeys()); diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java index 62140916fa8..e3f8b3e6df3 100644 --- a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java @@ -33,6 +33,7 @@ import org.eclipse.jetty.unixsocket.UnixSocketConnector; import org.eclipse.jetty.unixsocket.client.HttpClientTransportOverUnixSockets; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnJre; import org.junit.jupiter.api.condition.JRE; @@ -155,6 +156,18 @@ public class DistributionTests extends AbstractDistributionTest @Test @DisabledOnJre(JRE.JAVA_8) public void testSimpleWebAppWithJSPOverH2C() throws Exception + { + testSimpleWebAppWithJSPOverHTTP2(false); + } + + @Test + @DisabledOnJre(JRE.JAVA_8) + public void testSimpleWebAppWithJSPOverH2() throws Exception + { + testSimpleWebAppWithJSPOverHTTP2(true); + } + + private void testSimpleWebAppWithJSPOverHTTP2(boolean ssl) throws Exception { String jettyVersion = System.getProperty("jettyVersion"); DistributionTester distribution = DistributionTester.Builder.newInstance() @@ -164,7 +177,7 @@ public class DistributionTests extends AbstractDistributionTest String[] args1 = { "--create-startd", - "--add-to-start=http2c,jsp,deploy" + "--add-to-start=jsp,deploy," + (ssl ? "http2" : "http2c") }; try (DistributionTester.Run run1 = distribution.start(args1)) { @@ -175,13 +188,14 @@ public class DistributionTests extends AbstractDistributionTest distribution.installWarFile(war, "test"); int port = distribution.freePort(); - try (DistributionTester.Run run2 = distribution.start("jetty.http.port=" + port)) + String portProp = ssl ? "jetty.ssl.port" : "jetty.http.port"; + try (DistributionTester.Run run2 = distribution.start(portProp + "=" + port)) { assertTrue(run2.awaitConsoleLogsFor("Started @", 10, TimeUnit.SECONDS)); HTTP2Client h2Client = new HTTP2Client(); - startHttpClient(() -> new HttpClient(new HttpClientTransportOverHTTP2(h2Client), null)); - ContentResponse response = client.GET("http://localhost:" + port + "/test/index.jsp"); + startHttpClient(() -> new HttpClient(new HttpClientTransportOverHTTP2(h2Client), new SslContextFactory.Client(true))); + ContentResponse response = client.GET((ssl ? "https" : "http") + "://localhost:" + port + "/test/index.jsp"); assertEquals(HttpStatus.OK_200, response.getStatus()); assertThat(response.getContentAsString(), containsString("Hello")); assertThat(response.getContentAsString(), not(containsString("<%")));