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 82d6819e550..72f9e70d7c5 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 @@ -304,11 +304,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()); @@ -326,10 +326,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 @@ -347,12 +347,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()); @@ -361,7 +360,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); @@ -400,9 +399,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 cfd0178abd1..bb539061426 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 { 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 150977c8d05..1a475cdc432 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 5ae648a5f82..fafecd9d802 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 @@ -79,7 +79,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.setDryRun(true); @@ -177,4 +177,30 @@ public class TestQuickStart assertEquals("ascii", webapp.getDefaultRequestCharacterEncoding()); assertEquals("utf-16", webapp.getDefaultResponseCharacterEncoding()); } + + @Test + public void testListenersNotCalledInPreConfigure() throws Exception + { + File quickstartXml = new File(webInf, "quickstart-web.xml"); + assertFalse(quickstartXml.exists()); + + Server server = new Server(); + + WebAppContext quickstart = new WebAppContext(); + quickstart.addConfiguration(new QuickStartConfiguration()); + quickstart.setAttribute(QuickStartConfiguration.MODE, QuickStartConfiguration.Mode.GENERATE); + quickstart.setAttribute(QuickStartConfiguration.ORIGIN_ATTRIBUTE, "origin"); + + //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()); + server.setHandler(quickstart); + server.setDryRun(true); + + server.start(); + assertTrue(quickstartXml.exists()); + assertEquals(0, FooContextListener.___initialized); + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 9bcfaa22a63..001b88fbfeb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -956,6 +956,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu protected void callContextInitialized(ServletContextListener l, ServletContextEvent e) { + if (getServer().isDryRun()) + return; + if (LOG.isDebugEnabled()) LOG.debug("contextInitialized: {}->{}", e, l); l.contextInitialized(e); @@ -963,6 +966,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu protected void callContextDestroyed(ServletContextListener l, ServletContextEvent e) { + if (getServer().isDryRun()) + return; + if (LOG.isDebugEnabled()) LOG.debug("contextDestroyed: {}->{}", e, l); l.contextDestroyed(e); diff --git a/pom.xml b/pom.xml index 9a824ca94c3..b45b993acb0 100644 --- a/pom.xml +++ b/pom.xml @@ -1287,7 +1287,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