Issue #1209 - IllegalStateException when HTTP/2 push is disabled.

Skipping filter logic when HTTP/2 push is disabled.
This commit is contained in:
Simone Bordet 2016-12-31 12:19:46 +01:00
parent e215b065df
commit c018eb5b92
2 changed files with 96 additions and 5 deletions

View File

@ -21,6 +21,8 @@ package org.eclipse.jetty.http2.client;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@ -42,6 +44,7 @@ import org.eclipse.jetty.http2.frames.DataFrame;
import org.eclipse.jetty.http2.frames.HeadersFrame;
import org.eclipse.jetty.http2.frames.PushPromiseFrame;
import org.eclipse.jetty.http2.frames.ResetFrame;
import org.eclipse.jetty.http2.frames.SettingsFrame;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlets.PushCacheFilter;
import org.eclipse.jetty.util.Callback;
@ -885,4 +888,91 @@ public class PushCacheFilterTest extends AbstractTest
Assert.assertTrue(primaryResponseLatch.await(5, TimeUnit.SECONDS));
Assert.assertFalse(pushLatch.await(1, TimeUnit.SECONDS));
}
@Test
public void testPushDisabled() throws Exception
{
final String primaryResource = "/primary.html";
final String secondaryResource = "/secondary.png";
final byte[] secondaryData = "SECONDARY".getBytes("UTF-8");
start(new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
String requestURI = req.getRequestURI();
ServletOutputStream output = resp.getOutputStream();
if (requestURI.endsWith(primaryResource))
output.print("<html><head></head><body>PRIMARY</body></html>");
else if (requestURI.endsWith(secondaryResource))
output.write(secondaryData);
}
});
final Session session = newClient(new Session.Listener.Adapter()
{
@Override
public Map<Integer, Integer> onPreface(Session session)
{
Map<Integer, Integer> settings = new HashMap<>();
settings.put(SettingsFrame.ENABLE_PUSH, 0);
return settings;
}
});
// Request for the primary and secondary resource to build the cache.
final String referrerURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource;
HttpFields primaryFields = new HttpFields();
MetaData.Request primaryRequest = newRequest("GET", primaryResource, primaryFields);
final CountDownLatch warmupLatch = new CountDownLatch(1);
session.newStream(new HeadersFrame(primaryRequest, null, true), new Promise.Adapter<>(), new Stream.Listener.Adapter()
{
@Override
public void onData(Stream stream, DataFrame frame, Callback callback)
{
callback.succeeded();
if (frame.isEndStream())
{
// Request for the secondary resource.
HttpFields secondaryFields = new HttpFields();
secondaryFields.put(HttpHeader.REFERER, referrerURI);
MetaData.Request secondaryRequest = newRequest("GET", secondaryResource, secondaryFields);
session.newStream(new HeadersFrame(secondaryRequest, null, true), new Promise.Adapter<>(), new Stream.Listener.Adapter()
{
@Override
public void onData(Stream stream, DataFrame frame, Callback callback)
{
callback.succeeded();
warmupLatch.countDown();
}
});
}
}
});
Assert.assertTrue(warmupLatch.await(5, TimeUnit.SECONDS));
// Request again the primary resource, we should not get the secondary resource pushed.
primaryRequest = newRequest("GET", primaryResource, primaryFields);
final CountDownLatch primaryResponseLatch = new CountDownLatch(1);
final CountDownLatch pushLatch = new CountDownLatch(1);
session.newStream(new HeadersFrame(primaryRequest, null, true), new Promise.Adapter<>(), new Stream.Listener.Adapter()
{
@Override
public Stream.Listener onPush(Stream stream, PushPromiseFrame frame)
{
pushLatch.countDown();
return null;
}
@Override
public void onData(Stream stream, DataFrame frame, Callback callback)
{
callback.succeeded();
if (frame.isEndStream())
primaryResponseLatch.countDown();
}
});
Assert.assertFalse(pushLatch.await(1, TimeUnit.SECONDS));
Assert.assertTrue(primaryResponseLatch.await(5, TimeUnit.SECONDS));
}
}

View File

@ -116,9 +116,11 @@ public class PushCacheFilter implements Filter
public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) throws IOException, ServletException
{
HttpServletRequest request = (HttpServletRequest)req;
Request jettyRequest = Request.getBaseRequest(request);
if (HttpVersion.fromString(req.getProtocol()).getVersion() < 20 ||
!HttpMethod.GET.is(request.getMethod()))
if (HttpVersion.fromString(request.getProtocol()).getVersion() < 20 ||
!HttpMethod.GET.is(request.getMethod()) ||
!jettyRequest.isPushSupported())
{
chain.doFilter(req, resp);
return;
@ -127,7 +129,6 @@ public class PushCacheFilter implements Filter
long now = System.nanoTime();
// Iterating over fields is more efficient than multiple gets
Request jettyRequest = Request.getBaseRequest(request);
HttpFields fields = jettyRequest.getHttpFields();
boolean conditional = false;
String referrer = null;
@ -158,7 +159,7 @@ public class PushCacheFilter implements Filter
}
if (LOG.isDebugEnabled())
LOG.debug("{} {} referrer={} conditional={} synthetic={}", request.getMethod(), request.getRequestURI(), referrer, conditional, jettyRequest.isPush());
LOG.debug("{} {} referrer={} conditional={}", request.getMethod(), request.getRequestURI(), referrer, conditional);
String path = URIUtil.addPaths(request.getServletPath(), request.getPathInfo());
String query = request.getQueryString();
@ -256,7 +257,7 @@ public class PushCacheFilter implements Filter
}
// Push associated resources.
if (!jettyRequest.isPush() && !conditional && !primaryResource._associated.isEmpty())
if (!conditional && !primaryResource._associated.isEmpty())
{
PushBuilder pushBuilder = jettyRequest.getPushBuilder();