Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Jetty 9.4.x 5555 npe named filter mapping #5559

Merged
merged 2 commits into from Nov 2, 2020
Merged

Jetty 9.4.x 5555 npe named filter mapping #5559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
365bab1
Fixes #5555 NPE if Filter of named servlet
gregw Nov 2, 2020
4c49ca5
More cleanup for #5555
gregw Nov 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 12 additions & 19 deletions jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
View file
Open in desktop
Expand Up @@ -521,21 +521,8 @@ public void doHandle(String target, Request baseRequest, HttpServletRequest requ
FilterChain chain = null;

// find the servlet
if (target.startsWith("/"))
{
if (servletHolder != null && _filterMappings != null && _filterMappings.length > 0)
chain = getFilterChain(baseRequest, target, servletHolder);
}
else
{
if (servletHolder != null)
{
if (_filterMappings != null && _filterMappings.length > 0)
{
chain = getFilterChain(baseRequest, null, servletHolder);
}
}
}
if (servletHolder != null && _filterMappings != null && _filterMappings.length > 0)
chain = getFilterChain(baseRequest, target.startsWith("/") ? target : null, servletHolder);

if (LOG.isDebugEnabled())
LOG.debug("chain={}", chain);
Expand Down Expand Up @@ -593,6 +580,7 @@ public MappedResource<ServletHolder> getMappedServlet(String target)

protected FilterChain getFilterChain(Request baseRequest, String pathInContext, ServletHolder servletHolder)
{
Objects.requireNonNull(servletHolder);
String key = pathInContext == null ? servletHolder.getName() : pathInContext;
int dispatch = FilterMapping.dispatch(baseRequest.getDispatcherType());

Expand All @@ -608,16 +596,20 @@ protected FilterChain getFilterChain(Request baseRequest, String pathInContext,
// The mappings lists have been reversed to make this simple and fast.
FilterChain chain = null;

if (servletHolder != null && _filterNameMappings != null && !_filterNameMappings.isEmpty())
if (_filterNameMappings != null && !_filterNameMappings.isEmpty())
{
if (_wildFilterNameMappings != null)
for (FilterMapping mapping : _wildFilterNameMappings)
chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain);

for (FilterMapping mapping : _filterNameMappings.get(servletHolder.getName()))
List<FilterMapping> nameMappings = _filterNameMappings.get(servletHolder.getName());
if (nameMappings != null)
{
if (mapping.appliesTo(dispatch))
chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain);
for (FilterMapping mapping : nameMappings)
{
if (mapping.appliesTo(dispatch))
chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain);
}
}
}

Expand Down Expand Up @@ -1622,6 +1614,7 @@ static class ChainEnd implements FilterChain

ChainEnd(ServletHolder holder)
{
Objects.requireNonNull(holder);
_servletHolder = holder;
}

Expand Down
118 changes: 118 additions & 0 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletHandlerTest.java
View file
Open in desktop
Expand Up @@ -18,18 +18,29 @@

package org.eclipse.jetty.servlet;

import java.io.IOException;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import javax.servlet.DispatcherType;
import javax.servlet.Filter;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSessionEvent;
import javax.servlet.http.HttpSessionListener;

import org.eclipse.jetty.http.pathmap.MappedResource;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.util.component.Container;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand Down Expand Up @@ -730,4 +741,111 @@ public void sessionCreated(HttpSessionEvent se)
assertTrue(removeResults.contains(sh1));
assertTrue(removeResults.contains(lh1));
}

@Test
public void testServletMappings() throws Exception
{
Server server = new Server();
ServletHandler handler = new ServletHandler();
server.setHandler(handler);
for (final String mapping : new String[] {"/", "/foo", "/bar/*", "*.bob"})
{
handler.addServletWithMapping(new ServletHolder(new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
resp.getOutputStream().println("mapping='" + mapping + "'");
}
}), mapping);
}
// add servlet with no mapping
handler.addServlet(new ServletHolder(new HttpServlet() {}));

LocalConnector connector = new LocalConnector(server);
server.addConnector(connector);

server.start();

assertThat(connector.getResponse("GET /default HTTP/1.0\r\n\r\n"), containsString("mapping='/'"));
assertThat(connector.getResponse("GET /foo HTTP/1.0\r\n\r\n"), containsString("mapping='/foo'"));
assertThat(connector.getResponse("GET /bar HTTP/1.0\r\n\r\n"), containsString("mapping='/bar/*'"));
assertThat(connector.getResponse("GET /bar/bob HTTP/1.0\r\n\r\n"), containsString("mapping='/bar/*'"));
assertThat(connector.getResponse("GET /bar/foo.bob HTTP/1.0\r\n\r\n"), containsString("mapping='/bar/*'"));
assertThat(connector.getResponse("GET /other/foo.bob HTTP/1.0\r\n\r\n"), containsString("mapping='*.bob'"));
}

@Test
public void testFilterMappings() throws Exception
{
Server server = new Server();
ServletHandler handler = new ServletHandler();
server.setHandler(handler);

ServletHolder foo = new ServletHolder(new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
resp.getOutputStream().println("FOO");
}
});
foo.setName("foo");
handler.addServletWithMapping(foo, "/foo/*");

ServletHolder def = new ServletHolder(new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
resp.getOutputStream().println("default");
}
});
def.setName("default");
handler.addServletWithMapping(def, "/");

for (final String mapping : new String[]{"/*", "/foo", "/bar/*", "*.bob"})
{
handler.addFilterWithMapping(new FilterHolder((TestFilter)(request, response, chain) ->
{
response.getOutputStream().print("path-" + mapping + "-");
chain.doFilter(request, response);
}), mapping, EnumSet.of(DispatcherType.REQUEST));
}

FilterHolder fooFilter = new FilterHolder((TestFilter)(request, response, chain) ->
{
response.getOutputStream().print("name-foo-");
chain.doFilter(request, response);
});
fooFilter.setName("fooFilter");
FilterMapping named = new FilterMapping();
named.setFilterHolder(fooFilter);
named.setServletName("foo");
handler.addFilter(fooFilter, named);

LocalConnector connector = new LocalConnector(server);
server.addConnector(connector);

server.start();

assertThat(connector.getResponse("GET /default HTTP/1.0\r\n\r\n"), containsString("path-/*-default"));
assertThat(connector.getResponse("GET /foo HTTP/1.0\r\n\r\n"), containsString("path-/*-path-/foo-name-foo-FOO"));
assertThat(connector.getResponse("GET /foo/bar HTTP/1.0\r\n\r\n"), containsString("path-/*-name-foo-FOO"));
assertThat(connector.getResponse("GET /foo/bar.bob HTTP/1.0\r\n\r\n"), containsString("path-/*-path-*.bob-name-foo-FOO"));
assertThat(connector.getResponse("GET /other.bob HTTP/1.0\r\n\r\n"), containsString("path-/*-path-*.bob-default"));
}

private interface TestFilter extends Filter
{
default void init(FilterConfig filterConfig) throws ServletException
{
}

@Override
default void destroy()
{
}
}

}