Skip to content

Commit

Permalink
Fixes #5555 NPE if Filter of named servlet
Browse files Browse the repository at this point in the history 
Fixed #5555 NPE if there is a filter with a servlet name mapping, but a request is received for a servlet without a name match.
Added more simple tests for servlet and filter mappings
  • Loading branch information
@gregw
gregw committed Nov 2, 2020
1 parent dadd299 commit 365bab1
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 3 deletions.
10 changes: 7 additions & 3 deletions jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,10 +614,14 @@ protected FilterChain getFilterChain(Request baseRequest, String pathInContext,
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
118 changes: 118 additions & 0 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletHandlerTest.java
View file
Original file line number Diff line number Diff line change
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()
{
}
}

}

0 comments on commit 365bab1

Please sign in to comment.