From d0ecfbfd5677b49f5a25fa92cceb5eb396118b90 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Sat, 30 May 2020 12:46:21 +0200 Subject: [PATCH 1/3] Issue #4921 (#4922) Signed-off-by: Jan Bartel --- .../jetty/quickstart/QuickStartWebApp.java | 24 ++++++++- .../jetty/quickstart/FooContextListener.java | 18 +++++-- .../jetty/quickstart/TestQuickStart.java | 49 ++++++++++++++++++- 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartWebApp.java b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartWebApp.java index cf31de6fda2..01ccf550102 100644 --- a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartWebApp.java +++ b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartWebApp.java @@ -120,7 +120,29 @@ public class QuickStartWebApp extends WebAppContext { _generateOrigin = generateOrigin; } - + + /** + * Never call any listeners unless we are fully + * starting the webapp. + */ + @Override + public void contextInitialized() throws Exception + { + if (_startWebapp) + super.contextInitialized(); + } + + /** + * Never call any listeners unless we are fully + * starting the webapp. + */ + @Override + public void contextDestroyed() throws Exception + { + if (_startWebapp) + super.contextDestroyed(); + } + @Override protected void startWebapp() throws Exception { diff --git a/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/FooContextListener.java b/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/FooContextListener.java index f91cece5c32..2b7091d22bb 100644 --- a/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/FooContextListener.java +++ b/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/FooContextListener.java @@ -34,21 +34,31 @@ import static org.junit.jupiter.api.Assertions.assertTrue; */ public class FooContextListener implements ServletContextListener { + static int ___initialized; + static int __destroyed; + @Override public void contextInitialized(ServletContextEvent sce) { + ++___initialized; + ServletRegistration defaultRego = sce.getServletContext().getServletRegistration("default"); Collection mappings = defaultRego.getMappings(); assertThat("/", is(in(mappings))); - Set otherMappings = sce.getServletContext().getServletRegistration("foo").addMapping("/"); - assertTrue(otherMappings.isEmpty()); - Collection fooMappings = sce.getServletContext().getServletRegistration("foo").getMappings(); - assertThat("/", is(in(fooMappings))); + ServletRegistration rego = sce.getServletContext().getServletRegistration("foo"); + if (rego != null) + { + Set otherMappings = rego.addMapping("/"); + assertTrue(otherMappings.isEmpty()); + Collection fooMappings = rego.getMappings(); + assertThat("/", is(in(fooMappings))); + } } @Override public void contextDestroyed(ServletContextEvent sce) { + ++__destroyed; } } diff --git a/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java b/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java index 06d5dfe1186..51c9817b694 100644 --- a/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java +++ b/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java @@ -25,6 +25,7 @@ import org.eclipse.jetty.servlet.ListenerHolder; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -69,7 +70,7 @@ public class TestQuickStart fooHolder.setName("foo"); quickstart.getServletHandler().addServlet(fooHolder); ListenerHolder lholder = new ListenerHolder(); - lholder.setListener(new FooContextListener()); + lholder.setClassName("org.eclipse.jetty.quickstart.FooContextListener"); quickstart.getServletHandler().addListener(lholder); server.setHandler(quickstart); server.start(); @@ -92,4 +93,50 @@ public class TestQuickStart assertEquals("foo", sh.getName()); server.stop(); } + + @Test + public void testListenersNotCalledInPreConfigure() throws Exception + { + File quickstartXml = new File(webInf, "quickstart-web.xml"); + assertFalse(quickstartXml.exists()); + + Server server = new Server(); + + QuickStartWebApp quickstart = new QuickStartWebApp(); + + //add a listener directly to the ContextHandler so it is there when we start - + //if you add them to the ServletHandler (like StandardDescriptorProcessor does) + //then they are not added to the ContextHandler in a pre-generate. + quickstart.addEventListener(new FooContextListener()); + quickstart.setResourceBase(testDir.getAbsolutePath()); + quickstart.setPreconfigure(true); + quickstart.setGenerateOrigin(true); + server.setHandler(quickstart); + server.start(); + server.stop(); + assertTrue(quickstartXml.exists()); + assertEquals(0, FooContextListener.___initialized); + } + + @Test + public void testListenersCalledIfGenerateWithStart() throws Exception + { + File quickstartXml = new File(webInf, "quickstart-web.xml"); + assertFalse(quickstartXml.exists()); + + Server server = new Server(); + + QuickStartWebApp quickstart = new QuickStartWebApp(); + quickstart.setAutoPreconfigure(true); //generate AND start the webapp + + //add a listener directly to the ContextHandler so it is there when we start + quickstart.addEventListener(new FooContextListener()); + quickstart.setResourceBase(testDir.getAbsolutePath()); + quickstart.setGenerateOrigin(true); + server.setHandler(quickstart); + server.start(); + server.stop(); + assertTrue(quickstartXml.exists()); + assertEquals(1, FooContextListener.___initialized); + } } From ab0d5e7121eb2e56b629969a8e46670320808896 Mon Sep 17 00:00:00 2001 From: olivier lamy Date: Sat, 30 May 2020 21:28:33 +1000 Subject: [PATCH 2/3] simplify release script to not call goals already triggered by eclipse-release profile Signed-off-by: olivier lamy --- pom.xml | 2 +- scripts/release-jetty.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 91559f53c20..e2b8939c55f 100644 --- a/pom.xml +++ b/pom.xml @@ -1348,7 +1348,7 @@ attach-sources - jar + jar-no-fork diff --git a/scripts/release-jetty.sh b/scripts/release-jetty.sh index 408a639c68b..08008893ce5 100755 --- a/scripts/release-jetty.sh +++ b/scripts/release-jetty.sh @@ -167,7 +167,7 @@ if proceedyn "Are you sure you want to release using above? (y/N)" n; then # This is equivalent to 'mvn release:perform' if proceedyn "Build/Deploy from tag $TAG_NAME? (Y/n)" y; then git checkout $TAG_NAME - mvn clean package source:jar javadoc:jar gpg:sign javadoc:aggregate-jar deploy \ + mvn clean package javadoc:aggregate-jar deploy \ -Peclipse-release $DEPLOY_OPTS reportMavenTestFailures git checkout $GIT_BRANCH_ID From 646010e30905a3eda2d0a1b19e84ba179e69c87d Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 1 Jun 2020 14:12:50 +0200 Subject: [PATCH 3/3] Jetty 9.4.x #4890 do not index large fields (#4927) * Issue #4890 Large HTTP2 Fields encoded When choosing a strategy to encode a HTTP2 field, do not use indexed if the field is larger than the dynamic table. Signed-off-by: Greg Wilkins * Issue #4890 Large HTTP2 Fields encoded Fixed checkstyle in test Signed-off-by: Greg Wilkins * Issue #4890 Large HTTP2 Fields encoded Only index 0 content-length Signed-off-by: Greg Wilkins * Issue #4890 Large HTTP2 Fields encoded Fixed comments Signed-off-by: Greg Wilkins * Issue #4890 Large HTTP2 Fields encoded Don't parse int Signed-off-by: Greg Wilkins --- .../jetty/http2/hpack/HpackEncoder.java | 23 +++++---- .../jetty/http2/hpack/HpackEncoderTest.java | 49 +++++++++++++++++++ 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java index 0788806b34b..444b100afde 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java @@ -299,11 +299,11 @@ public class HpackEncoder String encoding = null; - // Is there an entry for the field? + // Is there an index entry for the field? Entry entry = _context.get(field); if (entry != null) { - // Known field entry, so encode it as indexed + // This is a known indexed field, send as static or dynamic indexed. if (entry.isStatic()) { buffer.put(((StaticEntry)entry).getEncodedField()); @@ -321,10 +321,10 @@ public class HpackEncoder } else { - // Unknown field entry, so we will have to send literally. + // Unknown field entry, so we will have to send literally, but perhaps add an index. final boolean indexed; - // But do we know it's name? + // Do we know its name? HttpHeader header = field.getHeader(); // Select encoding strategy @@ -342,12 +342,11 @@ public class HpackEncoder if (_debug) encoding = indexed ? "PreEncodedIdx" : "PreEncoded"; } - // has the custom header name been seen before? - else if (name == null) + else if (name == null && fieldSize < _context.getMaxDynamicTableSize()) { - // unknown name and value, so let's index this just in case it is - // the first time we have seen a custom name or a custom field. - // unless the name is changing, this is worthwhile + // unknown name and value that will fit in dynamic table, so let's index + // this just in case it is the first time we have seen a custom name or a + // custom field. Unless the name is once only, this is worthwhile indexed = true; encodeName(buffer, (byte)0x40, 6, field.getName(), null); encodeValue(buffer, true, field.getValue()); @@ -356,7 +355,7 @@ public class HpackEncoder } else { - // known custom name, but unknown value. + // Known name, but different value. // This is probably a custom field with changing value, so don't index. indexed = false; encodeName(buffer, (byte)0x00, 4, field.getName(), null); @@ -395,9 +394,9 @@ public class HpackEncoder (huffman ? "HuffV" : "LitV") + (neverIndex ? "!!Idx" : "!Idx"); } - else if (fieldSize >= _context.getMaxDynamicTableSize() || header == HttpHeader.CONTENT_LENGTH && field.getValue().length() > 2) + else if (fieldSize >= _context.getMaxDynamicTableSize() || header == HttpHeader.CONTENT_LENGTH && !"0".equals(field.getValue())) { - // Non indexed if field too large or a content length for 3 digits or more + // The field is too large or a non zero content length, so do not index. indexed = false; encodeName(buffer, (byte)0x00, 4, header.asString(), name); encodeValue(buffer, true, field.getValue()); diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java index e60ea65ba11..f9d8367dde5 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java @@ -22,6 +22,7 @@ import java.nio.ByteBuffer; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.util.BufferUtil; @@ -145,6 +146,54 @@ public class HpackEncoderTest assertEquals(5, encoder.getHpackContext().size()); } + @Test + public void testLargeFieldsNotIndexed() + { + HpackEncoder encoder = new HpackEncoder(38 * 5); + HpackContext ctx = encoder.getHpackContext(); + + ByteBuffer buffer = BufferUtil.allocate(4096); + + // Index little fields + int pos = BufferUtil.flipToFill(buffer); + encoder.encode(buffer, new HttpField("Name", "Value")); + BufferUtil.flipToFlush(buffer, pos); + int dynamicTableSize = ctx.getDynamicTableSize(); + assertThat(dynamicTableSize, Matchers.greaterThan(0)); + + // Do not index big field + StringBuilder largeName = new StringBuilder("largeName-"); + String filler = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"; + while (largeName.length() < ctx.getMaxDynamicTableSize()) + largeName.append(filler, 0, Math.min(filler.length(), ctx.getMaxDynamicTableSize() - largeName.length())); + pos = BufferUtil.flipToFill(buffer); + encoder.encode(buffer, new HttpField(largeName.toString(), "Value")); + BufferUtil.flipToFlush(buffer, pos); + assertThat(ctx.getDynamicTableSize(), Matchers.is(dynamicTableSize)); + } + + @Test + public void testIndexContentLength() + { + HpackEncoder encoder = new HpackEncoder(38 * 5); + HpackContext ctx = encoder.getHpackContext(); + + ByteBuffer buffer = BufferUtil.allocate(4096); + + // Index zero content length + int pos = BufferUtil.flipToFill(buffer); + encoder.encode(buffer, new HttpField(HttpHeader.CONTENT_LENGTH, "0")); + BufferUtil.flipToFlush(buffer, pos); + int dynamicTableSize = ctx.getDynamicTableSize(); + assertThat(dynamicTableSize, Matchers.greaterThan(0)); + + // Do not index non zero content length + pos = BufferUtil.flipToFill(buffer); + encoder.encode(buffer, new HttpField(HttpHeader.CONTENT_LENGTH, "42")); + BufferUtil.flipToFlush(buffer, pos); + assertThat(ctx.getDynamicTableSize(), Matchers.is(dynamicTableSize)); + } + @Test public void testNeverIndexSetCookie() throws Exception {