Merge pull request #3360 from eclipse/jetty-10.0.x-3340_push_filter

Fixes #3340 - Update PushCacheFilter to use Servlet 4.0 APIs.
This commit is contained in:
Simone Bordet 2019-02-20 19:32:41 +01:00 committed by GitHub
commit a76e25e3eb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 72 additions and 97 deletions

View File

@ -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();

View File

@ -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>()
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))

View File

@ -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<String> 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<PrimaryResource> queue = new ArrayDeque<>();
queue.offer(primaryResource);

View File

@ -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<String, Target> _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<String, Long> timestamps = (ConcurrentHashMap<String, Long>)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<String, Long> timestamps = (ConcurrentHashMap<String, Long>)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<Target> 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