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

Fixes #5555 NPE if Filter of named servlet #5556

Merged
merged 1 commit into from
Nov 2, 2020
Merged

Fixes #5555 NPE if Filter of named servlet #5556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
View file
Open in desktop
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
Open in desktop
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()
{
}
}

}