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 a8d66323f72..f62ac44d2bd 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 @@ -18,10 +18,6 @@ package org.eclipse.jetty.http2.client; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.EnumSet; @@ -31,7 +27,6 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import javax.servlet.DispatcherType; -import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -56,9 +51,12 @@ import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlets.PushCacheFilter; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Promise; - import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class PushCacheFilterTest extends AbstractTest { private String contextPath = "/push"; @@ -86,11 +84,11 @@ public class PushCacheFilterTest extends AbstractTest { final String primaryResource = "/primary.html"; final String secondaryResource = "/secondary.png"; - final byte[] secondaryData = "SECONDARY".getBytes("UTF-8"); + final byte[] secondaryData = "SECONDARY".getBytes(StandardCharsets.UTF_8); start(new HttpServlet() { @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { String requestURI = req.getRequestURI(); ServletOutputStream output = resp.getOutputStream(); @@ -188,11 +186,11 @@ public class PushCacheFilterTest extends AbstractTest { final String primaryResource = "/primary.html"; final String secondaryResource = "/secondary.png"; - final byte[] secondaryData = "SECONDARY".getBytes("UTF-8"); + final byte[] secondaryData = "SECONDARY".getBytes(StandardCharsets.UTF_8); start(new HttpServlet() { @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { String requestURI = req.getRequestURI(); ServletOutputStream output = resp.getOutputStream(); @@ -276,11 +274,11 @@ public class PushCacheFilterTest extends AbstractTest { final String primaryResource = "/primary.html"; final String secondaryResource = "/secondary.png"; - final byte[] secondaryData = "SECONDARY".getBytes("UTF-8"); + final byte[] secondaryData = "SECONDARY".getBytes(StandardCharsets.UTF_8); start(new HttpServlet() { @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { String requestURI = req.getRequestURI(); ServletOutputStream output = resp.getOutputStream(); @@ -385,7 +383,7 @@ public class PushCacheFilterTest extends AbstractTest start(new HttpServlet() { @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { String requestURI = request.getRequestURI(); final ServletOutputStream output = response.getOutputStream(); @@ -470,7 +468,7 @@ public class PushCacheFilterTest extends AbstractTest start(new HttpServlet() { @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { String requestURI = request.getRequestURI(); final ServletOutputStream output = response.getOutputStream(); @@ -654,7 +652,7 @@ public class PushCacheFilterTest extends AbstractTest start(new HttpServlet() { @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { ServletOutputStream output = response.getOutputStream(); String credentials = request.getParameter("credentials"); @@ -749,7 +747,7 @@ public class PushCacheFilterTest extends AbstractTest start(new HttpServlet() { @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void doGet(HttpServletRequest request, HttpServletResponse response) { String requestURI = request.getRequestURI(); if (requestURI.endsWith(primaryResource)) @@ -845,11 +843,11 @@ public class PushCacheFilterTest extends AbstractTest { final String primaryResource = "/primary.html"; final String secondaryResource = "/secondary.png"; - final byte[] secondaryData = "SECONDARY".getBytes("UTF-8"); + final byte[] secondaryData = "SECONDARY".getBytes(StandardCharsets.UTF_8); start(new HttpServlet() { @Override - protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException { String requestURI = req.getRequestURI(); ServletOutputStream output = resp.getOutputStream(); @@ -931,11 +929,11 @@ public class PushCacheFilterTest extends AbstractTest { final String primaryResource = "/primary.html"; final String secondaryResource = "/secondary.png"; - final byte[] secondaryData = "SECONDARY".getBytes("UTF-8"); + final byte[] secondaryData = "SECONDARY".getBytes(StandardCharsets.UTF_8); start(new HttpServlet() { @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { String requestURI = req.getRequestURI(); ServletOutputStream output = resp.getOutputStream(); diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/PushedResourcesTest.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/PushedResourcesTest.java index 8b38f9e6b2c..c2b89f2e089 100644 --- a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/PushedResourcesTest.java +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/PushedResourcesTest.java @@ -18,16 +18,11 @@ package org.eclipse.jetty.http2.client.http; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.IOException; import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -50,9 +45,12 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Promise; - import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class PushedResourcesTest extends AbstractTest { @Test @@ -67,7 +65,7 @@ public class PushedResourcesTest extends AbstractTest { HttpURI pushURI = new HttpURI("http://localhost:" + connector.getLocalPort() + pushPath); MetaData.Request pushRequest = new MetaData.Request(HttpMethod.GET.asString(), pushURI, HttpVersion.HTTP_2, new HttpFields()); - stream.push(new PushPromiseFrame(stream.getId(), 0, pushRequest), new Promise.Adapter() + stream.push(new PushPromiseFrame(stream.getId(), 0, pushRequest), new Promise.Adapter<>() { @Override public void succeeded(Stream pushStream) @@ -114,7 +112,7 @@ public class PushedResourcesTest extends AbstractTest start(new AbstractHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { baseRequest.setHandled(true); if (target.equals(path1)) @@ -181,7 +179,7 @@ public class PushedResourcesTest extends AbstractTest start(new AbstractHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { baseRequest.setHandled(true); if (target.equals(oldPath)) 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 34bf519fd89..4203e93b462 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 @@ -23,6 +23,7 @@ import java.util.ArrayDeque; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Queue; import java.util.Set; @@ -41,14 +42,11 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.PushBuilder; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; -import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; @@ -121,11 +119,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); + PushBuilder pushBuilder = request.newPushBuilder(); if (HttpVersion.fromString(request.getProtocol()).getVersion() < 20 || !HttpMethod.GET.is(request.getMethod()) || - !jettyRequest.isPushSupported()) + pushBuilder == null) { chain.doFilter(req, resp); return; @@ -133,33 +131,22 @@ public class PushCacheFilter implements Filter long now = System.nanoTime(); - // Iterating over fields is more efficient than multiple gets - HttpFields fields = jettyRequest.getHttpFields(); boolean conditional = false; String referrer = null; - loop: - for (int i = 0; i < fields.size(); i++) + List headerNames = Collections.list(request.getHeaderNames()); + for (String headerName : headerNames) { - HttpField field = fields.getField(i); - HttpHeader header = field.getHeader(); - if (header == null) - continue; - - switch (header) + if (HttpHeader.IF_MATCH.is(headerName) || + HttpHeader.IF_MODIFIED_SINCE.is(headerName) || + HttpHeader.IF_NONE_MATCH.is(headerName) || + HttpHeader.IF_UNMODIFIED_SINCE.is(headerName)) { - case IF_MATCH: - case IF_MODIFIED_SINCE: - case IF_NONE_MATCH: - case IF_UNMODIFIED_SINCE: - conditional = true; - break loop; - - case REFERER: - referrer = field.getValue(); - break; - - default: - break; + conditional = true; + break; + } + else if (HttpHeader.REFERER.is(headerName)) + { + referrer = request.getHeader(headerName); } } @@ -274,8 +261,6 @@ public class PushCacheFilter implements Filter // Push associated resources. if (!conditional && !primaryResource._associated.isEmpty()) { - PushBuilder pushBuilder = jettyRequest.newPushBuilder(); - // Breadth-first push of associated resources. Queue queue = new ArrayDeque<>(); queue.offer(primaryResource); diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/PushSessionCacheFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/PushSessionCacheFilter.java index e682d34dffa..b68983d7142 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/PushSessionCacheFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/PushSessionCacheFilter.java @@ -23,6 +23,7 @@ import java.util.ArrayDeque; import java.util.Queue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -32,20 +33,21 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletRequestEvent; import javax.servlet.ServletRequestListener; import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; import javax.servlet.http.PushBuilder; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpURI; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; public class PushSessionCacheFilter implements Filter { - private static final String TARGET_ATTR = "PushCacheFilter.target"; - private static final String TIMESTAMP_ATTR = "PushCacheFilter.timestamp"; + private static final String RESPONSE_ATTR = "PushSessionCacheFilter.response"; + private static final String TARGET_ATTR = "PushSessionCacheFilter.target"; + private static final String TIMESTAMP_ATTR = "PushSessionCacheFilter.timestamp"; private static final Logger LOG = Log.getLogger(PushSessionCacheFilter.class); private final ConcurrentMap _cache = new ConcurrentHashMap<>(); private long _associateDelay = 5000L; @@ -56,39 +58,29 @@ public class PushSessionCacheFilter implements Filter if (config.getInitParameter("associateDelay") != null) _associateDelay = Long.parseLong(config.getInitParameter("associateDelay")); - // Add a listener that is used to collect information about associated resource, - // etags and modified dates + // Add a listener that is used to collect information + // about associated resource, etags and modified dates. config.getServletContext().addListener(new ServletRequestListener() { // Collect information when request is destroyed. @Override public void requestDestroyed(ServletRequestEvent sre) { - Request request = Request.getBaseRequest(sre.getServletRequest()); + HttpServletRequest request = (HttpServletRequest)sre.getServletRequest(); Target target = (Target)request.getAttribute(TARGET_ATTR); if (target == null) return; - // Update conditional data - Response response = request.getResponse(); - target._etag = response.getHttpFields().get(HttpHeader.ETAG); - target._lastModified = response.getHttpFields().get(HttpHeader.LAST_MODIFIED); + // Update conditional data. + HttpServletResponse response = (HttpServletResponse)request.getAttribute(RESPONSE_ATTR); + target._etag = response.getHeader(HttpHeader.ETAG.asString()); + target._lastModified = response.getHeader(HttpHeader.LAST_MODIFIED.asString()); - // Don't associate pushes - if (request.isPush()) - { - if (LOG.isDebugEnabled()) - LOG.debug("Pushed {} for {}", request.getResponse().getStatus(), request.getRequestURI()); - return; - } - else if (LOG.isDebugEnabled()) - { - LOG.debug("Served {} for {}", request.getResponse().getStatus(), request.getRequestURI()); - } + if (LOG.isDebugEnabled()) + LOG.debug("Served {} for {}", response.getStatus(), request.getRequestURI()); // Does this request have a referer? - String referer = request.getHttpFields().get(HttpHeader.REFERER); - + String referer = request.getHeader(HttpHeader.REFERER.asString()); if (referer != null) { // Is the referer from this contexts? @@ -99,9 +91,10 @@ public class PushSessionCacheFilter implements Filter if (referer_target != null) { HttpSession session = request.getSession(); + @SuppressWarnings("unchecked") ConcurrentHashMap timestamps = (ConcurrentHashMap)session.getAttribute(TIMESTAMP_ATTR); Long last = timestamps.get(referer_target._path); - if (last != null && (System.currentTimeMillis() - last) < _associateDelay) + if (last != null && TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - last) < _associateDelay) { if (referer_target._associated.putIfAbsent(target._path, target) == null) { @@ -122,16 +115,16 @@ public class PushSessionCacheFilter implements Filter } @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException + public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) throws IOException, ServletException { - // Get Jetty request as these APIs are not yet standard - Request baseRequest = Request.getBaseRequest(request); - String uri = baseRequest.getRequestURI(); + req.setAttribute(RESPONSE_ATTR, resp); + HttpServletRequest request = (HttpServletRequest)req; + String uri = request.getRequestURI(); if (LOG.isDebugEnabled()) - LOG.debug("{} {} push={}", baseRequest.getMethod(), uri, baseRequest.isPush()); + LOG.debug("{} {}", request.getMethod(), uri); - HttpSession session = baseRequest.getSession(true); + HttpSession session = request.getSession(true); // find the target for this resource Target target = _cache.get(uri); @@ -144,26 +137,27 @@ public class PushSessionCacheFilter implements Filter request.setAttribute(TARGET_ATTR, target); // Set the timestamp for this resource in this session + @SuppressWarnings("unchecked") ConcurrentHashMap timestamps = (ConcurrentHashMap)session.getAttribute(TIMESTAMP_ATTR); if (timestamps == null) { timestamps = new ConcurrentHashMap<>(); session.setAttribute(TIMESTAMP_ATTR, timestamps); } - timestamps.put(uri, System.currentTimeMillis()); + timestamps.put(uri, System.nanoTime()); - // push any associated resources - if (baseRequest.isPushSupported() && !baseRequest.isPush() && !target._associated.isEmpty()) + // Push any associated resources. + PushBuilder builder = request.newPushBuilder(); + if (builder != null && !target._associated.isEmpty()) { - boolean conditional = baseRequest.getHttpFields().contains(HttpHeader.IF_NONE_MATCH) || - baseRequest.getHttpFields().contains(HttpHeader.IF_MODIFIED_SINCE); + boolean conditional = request.getHeader(HttpHeader.IF_NONE_MATCH.asString()) != null || + request.getHeader(HttpHeader.IF_MODIFIED_SINCE.asString()) != null; // Breadth-first push of associated resources. Queue queue = new ArrayDeque<>(); queue.offer(target); while (!queue.isEmpty()) { Target parent = queue.poll(); - PushBuilder builder = baseRequest.newPushBuilder(); builder.addHeader("X-Pusher", PushSessionCacheFilter.class.toString()); for (Target child : parent._associated.values()) { @@ -180,7 +174,7 @@ public class PushSessionCacheFilter implements Filter } } - chain.doFilter(request, response); + chain.doFilter(req, resp); } @Override