From 95c7e78eafbfa743728acfcecb3c28383d5c8665 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 5 Jan 2017 16:32:24 +0100 Subject: [PATCH] Issue #1220 - PushCacheFilter does not add the context path to pushed resources. Now taking into account the context path to cache and push resources. --- .../http2/client/PushCacheFilterTest.java | 79 +++++++++++++------ .../jetty/servlets/PushCacheFilter.java | 23 +++--- 2 files changed, 69 insertions(+), 33 deletions(-) diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/PushCacheFilterTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/PushCacheFilterTest.java index e9ce765e877..893485fa7cc 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/PushCacheFilterTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/PushCacheFilterTest.java @@ -33,9 +33,12 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HostPortHttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.api.Session; @@ -54,12 +57,26 @@ import org.junit.Test; public class PushCacheFilterTest extends AbstractTest { + private String contextPath = "/push"; + @Override protected void customizeContext(ServletContextHandler context) { + context.setContextPath(contextPath); context.addFilter(PushCacheFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST)); } + @Override + protected MetaData.Request newRequest(String method, String pathInfo, HttpFields fields) + { + return new MetaData.Request(method, HttpScheme.HTTP, new HostPortHttpField("localhost:" + connector.getLocalPort()), contextPath + servletPath + pathInfo, HttpVersion.HTTP_2, fields); + } + + private String newURI(String pathInfo) + { + return "http://localhost:" + connector.getLocalPort() + contextPath + servletPath + pathInfo; + } + @Test public void testPush() throws Exception { @@ -83,7 +100,7 @@ public class PushCacheFilterTest extends AbstractTest final Session session = newClient(new Session.Listener.Adapter()); // Request for the primary and secondary resource to build the cache. - final String referrerURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource; + final String referrerURI = newURI(primaryResource); HttpFields primaryFields = new HttpFields(); MetaData.Request primaryRequest = newRequest("GET", primaryResource, primaryFields); final CountDownLatch warmupLatch = new CountDownLatch(1); @@ -115,23 +132,16 @@ public class PushCacheFilterTest extends AbstractTest // Request again the primary resource, we should get the secondary resource pushed. primaryRequest = newRequest("GET", primaryResource, primaryFields); - final CountDownLatch primaryResponseLatch = new CountDownLatch(1); - final CountDownLatch pushLatch = new CountDownLatch(1); + final CountDownLatch primaryResponseLatch = new CountDownLatch(2); + final CountDownLatch pushLatch = new CountDownLatch(2); session.newStream(new HeadersFrame(primaryRequest, null, true), new Promise.Adapter<>(), new Stream.Listener.Adapter() { @Override - public Stream.Listener onPush(Stream stream, PushPromiseFrame frame) + public void onHeaders(Stream stream, HeadersFrame frame) { - return new Adapter() - { - @Override - public void onData(Stream stream, DataFrame frame, Callback callback) - { - callback.succeeded(); - if (frame.isEndStream()) - pushLatch.countDown(); - } - }; + MetaData.Response response = (MetaData.Response)frame.getMetaData(); + if (response.getStatus() == HttpStatus.OK_200) + primaryResponseLatch.countDown(); } @Override @@ -141,6 +151,29 @@ public class PushCacheFilterTest extends AbstractTest if (frame.isEndStream()) primaryResponseLatch.countDown(); } + + @Override + public Stream.Listener onPush(Stream stream, PushPromiseFrame frame) + { + return new Adapter() + { + @Override + public void onHeaders(Stream stream, HeadersFrame frame) + { + MetaData.Response response = (MetaData.Response)frame.getMetaData(); + if (response.getStatus() == HttpStatus.OK_200) + pushLatch.countDown(); + } + + @Override + public void onData(Stream stream, DataFrame frame, Callback callback) + { + callback.succeeded(); + if (frame.isEndStream()) + pushLatch.countDown(); + } + }; + } }); Assert.assertTrue(pushLatch.await(5, TimeUnit.SECONDS)); Assert.assertTrue(primaryResponseLatch.await(5, TimeUnit.SECONDS)); @@ -257,7 +290,7 @@ public class PushCacheFilterTest extends AbstractTest final Session session = newClient(new Session.Listener.Adapter()); // Request for the primary and secondary resource to build the cache. - final String primaryURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource; + final String primaryURI = newURI(primaryResource); HttpFields primaryFields = new HttpFields(); MetaData.Request primaryRequest = newRequest("GET", primaryResource, primaryFields); final CountDownLatch warmupLatch = new CountDownLatch(1); @@ -360,7 +393,7 @@ public class PushCacheFilterTest extends AbstractTest final Session session = newClient(new Session.Listener.Adapter()); // Request for the primary and secondary resource to build the cache. - final String primaryURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource; + final String primaryURI = newURI(primaryResource); HttpFields primaryFields = new HttpFields(); MetaData.Request primaryRequest = newRequest("GET", primaryResource, primaryFields); final CountDownLatch warmupLatch = new CountDownLatch(1); @@ -451,7 +484,7 @@ public class PushCacheFilterTest extends AbstractTest final Session session = newClient(new Session.Listener.Adapter()); // Request for the primary, secondary and tertiary resource to build the cache. - final String primaryURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource; + final String primaryURI = newURI(primaryResource); HttpFields primaryFields = new HttpFields(); MetaData.Request primaryRequest = newRequest("GET", primaryResource, primaryFields); final CountDownLatch warmupLatch = new CountDownLatch(2); @@ -464,7 +497,7 @@ public class PushCacheFilterTest extends AbstractTest if (frame.isEndStream()) { // Request for the secondary resources. - String secondaryURI1 = "http://localhost:" + connector.getLocalPort() + servletPath + secondaryResource1; + String secondaryURI1 = newURI(secondaryResource1); HttpFields secondaryFields1 = new HttpFields(); secondaryFields1.put(HttpHeader.REFERER, primaryURI); MetaData.Request secondaryRequest1 = newRequest("GET", secondaryResource1, secondaryFields1); @@ -636,7 +669,7 @@ public class PushCacheFilterTest extends AbstractTest } } }); - final String primaryURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource; + final String primaryURI = newURI(primaryResource); final Session session = newClient(new Session.Listener.Adapter()); @@ -733,7 +766,7 @@ public class PushCacheFilterTest extends AbstractTest final Session session = newClient(new Session.Listener.Adapter()); // Request for the primary and secondary resource to build the cache. - final String primaryURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource; + final String primaryURI = newURI(primaryResource); HttpFields primaryFields = new HttpFields(); MetaData.Request primaryRequest = newRequest("GET", primaryResource, primaryFields); final CountDownLatch warmupLatch = new CountDownLatch(1); @@ -776,7 +809,7 @@ public class PushCacheFilterTest extends AbstractTest MetaData metaData = frame.getMetaData(); Assert.assertTrue(metaData instanceof MetaData.Request); MetaData.Request pushedRequest = (MetaData.Request)metaData; - Assert.assertEquals(servletPath + secondaryResource, pushedRequest.getURI().getPathQuery()); + Assert.assertEquals(contextPath + servletPath + secondaryResource, pushedRequest.getURI().getPathQuery()); return new Adapter() { @Override @@ -826,7 +859,7 @@ public class PushCacheFilterTest extends AbstractTest final Session session = newClient(new Session.Listener.Adapter()); // Request for the primary and secondary resource to build the cache. - final String referrerURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource; + final String referrerURI = newURI(primaryResource); HttpFields primaryFields = new HttpFields(); MetaData.Request primaryRequest = newRequest("GET", primaryResource, primaryFields); final CountDownLatch warmupLatch = new CountDownLatch(1); @@ -921,7 +954,7 @@ public class PushCacheFilterTest extends AbstractTest }); // Request for the primary and secondary resource to build the cache. - final String referrerURI = "http://localhost:" + connector.getLocalPort() + servletPath + primaryResource; + final String referrerURI = newURI(primaryResource); HttpFields primaryFields = new HttpFields(); MetaData.Request primaryRequest = newRequest("GET", primaryResource, primaryFields); final CountDownLatch warmupLatch = new CountDownLatch(1); diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/PushCacheFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/PushCacheFilter.java index 57cf4f5048d..49046824c76 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/PushCacheFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/PushCacheFilter.java @@ -49,7 +49,6 @@ import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.server.PushBuilder; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.StringUtil; -import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; @@ -161,7 +160,7 @@ public class PushCacheFilter implements Filter if (LOG.isDebugEnabled()) LOG.debug("{} {} referrer={} conditional={}", request.getMethod(), request.getRequestURI(), referrer, conditional); - String path = URIUtil.addPaths(request.getServletPath(), request.getPathInfo()); + String path = request.getRequestURI(); String query = request.getQueryString(); if (query != null) path += "?" + query; @@ -183,12 +182,11 @@ public class PushCacheFilter implements Filter String referrerPath = referrerURI.getPath(); if (referrerPath == null) referrerPath = "/"; - if (referrerPath.startsWith(request.getContextPath())) + if (referrerPath.startsWith(request.getContextPath() + "/")) { - String referrerPathNoContext = referrerPath.substring(request.getContextPath().length()); - if (!referrerPathNoContext.equals(path)) + if (!referrerPath.equals(path)) { - PrimaryResource primaryResource = _cache.get(referrerPathNoContext); + PrimaryResource primaryResource = _cache.get(referrerPath); if (primaryResource != null) { long primaryTimestamp = primaryResource._timestamp.get(); @@ -203,19 +201,19 @@ public class PushCacheFilter implements Filter if (associated.add(path)) { if (LOG.isDebugEnabled()) - LOG.debug("Associated {} to {}", path, referrerPathNoContext); + LOG.debug("Associated {} to {}", path, referrerPath); } } else { if (LOG.isDebugEnabled()) - LOG.debug("Not associated {} to {}, exceeded max associations of {}", path, referrerPathNoContext, _maxAssociations); + LOG.debug("Not associated {} to {}, exceeded max associations of {}", path, referrerPath, _maxAssociations); } } else { if (LOG.isDebugEnabled()) - LOG.debug("Not associated {} to {}, outside associate period of {}ms", path, referrerPathNoContext, _associatePeriod); + LOG.debug("Not associated {} to {}, outside associate period of {}ms", path, referrerPath, _associatePeriod); } } } @@ -223,9 +221,14 @@ public class PushCacheFilter implements Filter else { if (LOG.isDebugEnabled()) - LOG.debug("Not associated {} to {}, referring to self", path, referrerPathNoContext); + LOG.debug("Not associated {} to {}, referring to self", path, referrerPath); } } + else + { + if (LOG.isDebugEnabled()) + LOG.debug("Not associated {} to {}, different context", path, referrerPath); + } } } else