Fixes #6263 - Review URI encoding in ConcatServlet & WelcomeFilter.
Review URI encoding in ConcatServlet & WelcomeFilter and improve testing. Signed-off-by: Lachlan Roberts <lachlan@webtide.com> Signed-off-by: Simone Bordet <simone.bordet@gmail.com> Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
parent
8fee07aca8
commit
f58dbedcd0
|
@ -52,6 +52,7 @@ public interface HttpURI
|
||||||
{
|
{
|
||||||
SEGMENT,
|
SEGMENT,
|
||||||
SEPARATOR,
|
SEPARATOR,
|
||||||
|
ENCODING,
|
||||||
PARAM
|
PARAM
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -159,6 +160,11 @@ public interface HttpURI
|
||||||
*/
|
*/
|
||||||
boolean hasAmbiguousParameter();
|
boolean hasAmbiguousParameter();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return True if the URI has an encoded '%' character.
|
||||||
|
*/
|
||||||
|
boolean hasAmbiguousEncoding();
|
||||||
|
|
||||||
default URI toURI()
|
default URI toURI()
|
||||||
{
|
{
|
||||||
try
|
try
|
||||||
|
@ -386,6 +392,12 @@ public interface HttpURI
|
||||||
return _ambiguous.contains(Ambiguous.PARAM);
|
return _ambiguous.contains(Ambiguous.PARAM);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public boolean hasAmbiguousEncoding()
|
||||||
|
{
|
||||||
|
return _ambiguous.contains(Ambiguous.ENCODING);
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public String toString()
|
public String toString()
|
||||||
{
|
{
|
||||||
|
@ -727,6 +739,12 @@ public interface HttpURI
|
||||||
return _ambiguous.contains(Ambiguous.PARAM);
|
return _ambiguous.contains(Ambiguous.PARAM);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public boolean hasAmbiguousEncoding()
|
||||||
|
{
|
||||||
|
return _ambiguous.contains(Ambiguous.ENCODING);
|
||||||
|
}
|
||||||
|
|
||||||
public Mutable normalize()
|
public Mutable normalize()
|
||||||
{
|
{
|
||||||
HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme);
|
HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme);
|
||||||
|
@ -884,7 +902,7 @@ public interface HttpURI
|
||||||
int segment = 0; // the start of the current segment within the path
|
int segment = 0; // the start of the current segment within the path
|
||||||
boolean encoded = false; // set to true if the path contains % encoded characters
|
boolean encoded = false; // set to true if the path contains % encoded characters
|
||||||
boolean dot = false; // set to true if the path containers . or .. segments
|
boolean dot = false; // set to true if the path containers . or .. segments
|
||||||
int escapedSlash = 0; // state of parsing a %2f
|
int escapedTwo = 0; // state of parsing a %2<x>
|
||||||
int end = uri.length();
|
int end = uri.length();
|
||||||
for (int i = 0; i < end; i++)
|
for (int i = 0; i < end; i++)
|
||||||
{
|
{
|
||||||
|
@ -920,7 +938,7 @@ public interface HttpURI
|
||||||
break;
|
break;
|
||||||
case '%':
|
case '%':
|
||||||
encoded = true;
|
encoded = true;
|
||||||
escapedSlash = 1;
|
escapedTwo = 1;
|
||||||
mark = pathMark = segment = i;
|
mark = pathMark = segment = i;
|
||||||
state = State.PATH;
|
state = State.PATH;
|
||||||
break;
|
break;
|
||||||
|
@ -972,7 +990,7 @@ public interface HttpURI
|
||||||
case '%':
|
case '%':
|
||||||
// must have be in an encoded path
|
// must have be in an encoded path
|
||||||
encoded = true;
|
encoded = true;
|
||||||
escapedSlash = 1;
|
escapedTwo = 1;
|
||||||
state = State.PATH;
|
state = State.PATH;
|
||||||
break;
|
break;
|
||||||
case '#':
|
case '#':
|
||||||
|
@ -1120,19 +1138,24 @@ public interface HttpURI
|
||||||
break;
|
break;
|
||||||
case '%':
|
case '%':
|
||||||
encoded = true;
|
encoded = true;
|
||||||
escapedSlash = 1;
|
escapedTwo = 1;
|
||||||
break;
|
break;
|
||||||
case '2':
|
case '2':
|
||||||
escapedSlash = escapedSlash == 1 ? 2 : 0;
|
escapedTwo = escapedTwo == 1 ? 2 : 0;
|
||||||
break;
|
break;
|
||||||
case 'f':
|
case 'f':
|
||||||
case 'F':
|
case 'F':
|
||||||
if (escapedSlash == 2)
|
if (escapedTwo == 2)
|
||||||
_ambiguous.add(Ambiguous.SEPARATOR);
|
_ambiguous.add(Ambiguous.SEPARATOR);
|
||||||
escapedSlash = 0;
|
escapedTwo = 0;
|
||||||
|
break;
|
||||||
|
case '5':
|
||||||
|
if (escapedTwo == 2)
|
||||||
|
_ambiguous.add(Ambiguous.ENCODING);
|
||||||
|
escapedTwo = 0;
|
||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
escapedSlash = 0;
|
escapedTwo = 0;
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
|
|
|
@ -58,6 +58,10 @@ public final class UriCompliance implements ComplianceViolation.Mode
|
||||||
* Allow ambiguous path parameters within a URI segment e.g. <code>/foo/..;/bar</code>
|
* Allow ambiguous path parameters within a URI segment e.g. <code>/foo/..;/bar</code>
|
||||||
*/
|
*/
|
||||||
AMBIGUOUS_PATH_PARAMETER("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path parameter"),
|
AMBIGUOUS_PATH_PARAMETER("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path parameter"),
|
||||||
|
/**
|
||||||
|
* Allow ambiguous path encoding within a URI segment e.g. <code>/%2557EB-INF</code>
|
||||||
|
*/
|
||||||
|
AMBIGUOUS_PATH_ENCODING("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path encoding"),
|
||||||
/**
|
/**
|
||||||
* Allow Non canonical ambiguous paths. eg <code>/foo/x%2f%2e%2e%/bar</code> provided to applications as <code>/foo/x/../bar</code>
|
* Allow Non canonical ambiguous paths. eg <code>/foo/x%2f%2e%2e%/bar</code> provided to applications as <code>/foo/x/../bar</code>
|
||||||
*/
|
*/
|
||||||
|
@ -94,15 +98,15 @@ public final class UriCompliance implements ComplianceViolation.Mode
|
||||||
/**
|
/**
|
||||||
* The default compliance mode that extends RFC3986 compliance with additional violations to avoid most ambiguous URIs.
|
* The default compliance mode that extends RFC3986 compliance with additional violations to avoid most ambiguous URIs.
|
||||||
* This mode does allow {@link Violation#AMBIGUOUS_PATH_SEPARATOR}, but disallows
|
* This mode does allow {@link Violation#AMBIGUOUS_PATH_SEPARATOR}, but disallows
|
||||||
* {@link Violation#AMBIGUOUS_PATH_PARAMETER} and {@link Violation#AMBIGUOUS_PATH_SEGMENT}.
|
* {@link Violation#AMBIGUOUS_PATH_PARAMETER}, {@link Violation#AMBIGUOUS_PATH_SEGMENT} and {@link Violation#AMBIGUOUS_PATH_ENCODING}.
|
||||||
* Ambiguous paths are not allowed by {@link Violation#NON_CANONICAL_AMBIGUOUS_PATHS}.
|
* Ambiguous paths are not allowed by {@link Violation#NON_CANONICAL_AMBIGUOUS_PATHS}.
|
||||||
*/
|
*/
|
||||||
public static final UriCompliance DEFAULT = new UriCompliance("DEFAULT", of(Violation.AMBIGUOUS_PATH_SEPARATOR));
|
public static final UriCompliance DEFAULT = new UriCompliance("DEFAULT", of(Violation.AMBIGUOUS_PATH_SEPARATOR));
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* LEGACY compliance mode that models Jetty-9.4 behavior by allowing {@link Violation#AMBIGUOUS_PATH_SEGMENT} and {@link Violation#AMBIGUOUS_PATH_SEPARATOR}
|
* LEGACY compliance mode that models Jetty-9.4 behavior by allowing {@link Violation#AMBIGUOUS_PATH_SEGMENT}, {@link Violation#AMBIGUOUS_PATH_SEPARATOR} and {@link Violation#AMBIGUOUS_PATH_ENCODING}.
|
||||||
*/
|
*/
|
||||||
public static final UriCompliance LEGACY = new UriCompliance("LEGACY", of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_SEPARATOR));
|
public static final UriCompliance LEGACY = new UriCompliance("LEGACY", of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_SEPARATOR, Violation.AMBIGUOUS_PATH_ENCODING));
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Compliance mode that exactly follows RFC3986, including allowing all additional ambiguous URI Violations,
|
* Compliance mode that exactly follows RFC3986, including allowing all additional ambiguous URI Violations,
|
||||||
|
|
|
@ -1702,6 +1702,8 @@ public class Request implements HttpServletRequest
|
||||||
throw new BadMessageException("Ambiguous segment in URI");
|
throw new BadMessageException("Ambiguous segment in URI");
|
||||||
if (uri.hasAmbiguousParameter() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_PARAMETER)))
|
if (uri.hasAmbiguousParameter() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_PARAMETER)))
|
||||||
throw new BadMessageException("Ambiguous path parameter in URI");
|
throw new BadMessageException("Ambiguous path parameter in URI");
|
||||||
|
if (uri.hasAmbiguousEncoding() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING)))
|
||||||
|
throw new BadMessageException("Ambiguous path encoding in URI");
|
||||||
}
|
}
|
||||||
|
|
||||||
if (uri.isAbsolute() && uri.hasAuthority() && uri.getPath() != null)
|
if (uri.isAbsolute() && uri.hasAuthority() && uri.getPath() != null)
|
||||||
|
|
|
@ -428,7 +428,7 @@ public class ResourceService
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
RequestDispatcher dispatcher = context.getRequestDispatcher(welcome);
|
RequestDispatcher dispatcher = context.getRequestDispatcher(URIUtil.encodePath(welcome));
|
||||||
if (dispatcher != null)
|
if (dispatcher != null)
|
||||||
{
|
{
|
||||||
// Forward to the index
|
// Forward to the index
|
||||||
|
|
|
@ -1774,6 +1774,23 @@ public class RequestTest
|
||||||
containsString("pathInfo=/path/ambiguous/.././info")));
|
containsString("pathInfo=/path/ambiguous/.././info")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testAmbiguousEncoding() throws Exception
|
||||||
|
{
|
||||||
|
_handler._checker = (request, response) -> true;
|
||||||
|
String request = "GET /ambiguous/encoded/%25/path HTTP/1.0\r\n" +
|
||||||
|
"Host: whatever\r\n" +
|
||||||
|
"\r\n";
|
||||||
|
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT);
|
||||||
|
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400"));
|
||||||
|
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.LEGACY);
|
||||||
|
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
|
||||||
|
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986);
|
||||||
|
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
|
||||||
|
_connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE);
|
||||||
|
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testPushBuilder()
|
public void testPushBuilder()
|
||||||
{
|
{
|
||||||
|
|
|
@ -56,6 +56,7 @@ import org.eclipse.jetty.util.URIUtil;
|
||||||
* appropriate. This means that when not in development mode, the servlet must be
|
* appropriate. This means that when not in development mode, the servlet must be
|
||||||
* restarted before changed content will be served.</p>
|
* restarted before changed content will be served.</p>
|
||||||
*/
|
*/
|
||||||
|
@Deprecated
|
||||||
public class ConcatServlet extends HttpServlet
|
public class ConcatServlet extends HttpServlet
|
||||||
{
|
{
|
||||||
private boolean _development;
|
private boolean _development;
|
||||||
|
@ -120,7 +121,8 @@ public class ConcatServlet extends HttpServlet
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
RequestDispatcher dispatcher = getServletContext().getRequestDispatcher(path);
|
// Use the original string and not the decoded path as the Dispatcher will decode again.
|
||||||
|
RequestDispatcher dispatcher = getServletContext().getRequestDispatcher(part);
|
||||||
if (dispatcher != null)
|
if (dispatcher != null)
|
||||||
dispatchers.add(dispatcher);
|
dispatchers.add(dispatcher);
|
||||||
}
|
}
|
||||||
|
|
|
@ -22,6 +22,8 @@ import javax.servlet.ServletRequest;
|
||||||
import javax.servlet.ServletResponse;
|
import javax.servlet.ServletResponse;
|
||||||
import javax.servlet.http.HttpServletRequest;
|
import javax.servlet.http.HttpServletRequest;
|
||||||
|
|
||||||
|
import org.eclipse.jetty.util.URIUtil;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Welcome Filter
|
* Welcome Filter
|
||||||
* This filter can be used to server an index file for a directory
|
* This filter can be used to server an index file for a directory
|
||||||
|
@ -36,6 +38,7 @@ import javax.servlet.http.HttpServletRequest;
|
||||||
*
|
*
|
||||||
* Requests to "/some/directory" will be redirected to "/some/directory/".
|
* Requests to "/some/directory" will be redirected to "/some/directory/".
|
||||||
*/
|
*/
|
||||||
|
@Deprecated
|
||||||
public class WelcomeFilter implements Filter
|
public class WelcomeFilter implements Filter
|
||||||
{
|
{
|
||||||
private String welcome;
|
private String welcome;
|
||||||
|
@ -56,7 +59,10 @@ public class WelcomeFilter implements Filter
|
||||||
{
|
{
|
||||||
String path = ((HttpServletRequest)request).getServletPath();
|
String path = ((HttpServletRequest)request).getServletPath();
|
||||||
if (welcome != null && path.endsWith("/"))
|
if (welcome != null && path.endsWith("/"))
|
||||||
request.getRequestDispatcher(path + welcome).forward(request, response);
|
{
|
||||||
|
String uriInContext = URIUtil.encodePath(URIUtil.addPaths(path, welcome));
|
||||||
|
request.getRequestDispatcher(uriInContext).forward(request, response);
|
||||||
|
}
|
||||||
else
|
else
|
||||||
chain.doFilter(request, response);
|
chain.doFilter(request, response);
|
||||||
}
|
}
|
||||||
|
|
|
@ -21,12 +21,15 @@ import java.io.StringReader;
|
||||||
import java.nio.charset.StandardCharsets;
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.nio.file.Files;
|
import java.nio.file.Files;
|
||||||
import java.nio.file.Path;
|
import java.nio.file.Path;
|
||||||
|
import java.util.stream.Stream;
|
||||||
import javax.servlet.RequestDispatcher;
|
import javax.servlet.RequestDispatcher;
|
||||||
import javax.servlet.ServletException;
|
import javax.servlet.ServletException;
|
||||||
import javax.servlet.http.HttpServlet;
|
import javax.servlet.http.HttpServlet;
|
||||||
import javax.servlet.http.HttpServletRequest;
|
import javax.servlet.http.HttpServletRequest;
|
||||||
import javax.servlet.http.HttpServletResponse;
|
import javax.servlet.http.HttpServletResponse;
|
||||||
|
|
||||||
|
import org.eclipse.jetty.http.UriCompliance;
|
||||||
|
import org.eclipse.jetty.server.HttpConnectionFactory;
|
||||||
import org.eclipse.jetty.server.LocalConnector;
|
import org.eclipse.jetty.server.LocalConnector;
|
||||||
import org.eclipse.jetty.server.Server;
|
import org.eclipse.jetty.server.Server;
|
||||||
import org.eclipse.jetty.servlet.ServletContextHandler;
|
import org.eclipse.jetty.servlet.ServletContextHandler;
|
||||||
|
@ -36,7 +39,12 @@ import org.eclipse.jetty.webapp.WebAppContext;
|
||||||
import org.junit.jupiter.api.AfterEach;
|
import org.junit.jupiter.api.AfterEach;
|
||||||
import org.junit.jupiter.api.BeforeEach;
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.junit.jupiter.params.ParameterizedTest;
|
||||||
|
import org.junit.jupiter.params.provider.Arguments;
|
||||||
|
import org.junit.jupiter.params.provider.MethodSource;
|
||||||
|
|
||||||
|
import static org.hamcrest.MatcherAssert.assertThat;
|
||||||
|
import static org.hamcrest.Matchers.startsWith;
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||||
|
@ -48,10 +56,11 @@ public class ConcatServletTest
|
||||||
private LocalConnector connector;
|
private LocalConnector connector;
|
||||||
|
|
||||||
@BeforeEach
|
@BeforeEach
|
||||||
public void prepareServer() throws Exception
|
public void prepareServer()
|
||||||
{
|
{
|
||||||
server = new Server();
|
server = new Server();
|
||||||
connector = new LocalConnector(server);
|
connector = new LocalConnector(server);
|
||||||
|
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986);
|
||||||
server.addConnector(connector);
|
server.addConnector(connector);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -73,7 +82,7 @@ public class ConcatServletTest
|
||||||
ServletHolder resourceServletHolder = new ServletHolder(new HttpServlet()
|
ServletHolder resourceServletHolder = new ServletHolder(new HttpServlet()
|
||||||
{
|
{
|
||||||
@Override
|
@Override
|
||||||
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
|
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException
|
||||||
{
|
{
|
||||||
String includedURI = (String)request.getAttribute(RequestDispatcher.INCLUDE_REQUEST_URI);
|
String includedURI = (String)request.getAttribute(RequestDispatcher.INCLUDE_REQUEST_URI);
|
||||||
response.getOutputStream().println(includedURI);
|
response.getOutputStream().println(includedURI);
|
||||||
|
@ -107,7 +116,7 @@ public class ConcatServletTest
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testWEBINFResourceIsNotServed() throws Exception
|
public void testDirectoryNotAccessible() throws Exception
|
||||||
{
|
{
|
||||||
File directoryFile = MavenTestingUtils.getTargetTestingDir();
|
File directoryFile = MavenTestingUtils.getTargetTestingDir();
|
||||||
Path directoryPath = directoryFile.toPath();
|
Path directoryPath = directoryFile.toPath();
|
||||||
|
@ -129,9 +138,8 @@ public class ConcatServletTest
|
||||||
// Verify that I can get the file programmatically, as required by the spec.
|
// Verify that I can get the file programmatically, as required by the spec.
|
||||||
assertNotNull(context.getServletContext().getResource("/WEB-INF/one.js"));
|
assertNotNull(context.getServletContext().getResource("/WEB-INF/one.js"));
|
||||||
|
|
||||||
// Having a path segment and then ".." triggers a special case
|
// Make sure ConcatServlet cannot see file system files.
|
||||||
// that the ConcatServlet must detect and avoid.
|
String uri = contextPath + concatPath + "?/trick/../../" + directoryFile.getName();
|
||||||
String uri = contextPath + concatPath + "?/trick/../WEB-INF/one.js";
|
|
||||||
String request =
|
String request =
|
||||||
"GET " + uri + " HTTP/1.1\r\n" +
|
"GET " + uri + " HTTP/1.1\r\n" +
|
||||||
"Host: localhost\r\n" +
|
"Host: localhost\r\n" +
|
||||||
|
@ -139,35 +147,59 @@ public class ConcatServletTest
|
||||||
"\r\n";
|
"\r\n";
|
||||||
String response = connector.getResponse(request);
|
String response = connector.getResponse(request);
|
||||||
assertTrue(response.startsWith("HTTP/1.1 404 "));
|
assertTrue(response.startsWith("HTTP/1.1 404 "));
|
||||||
|
}
|
||||||
|
|
||||||
// Make sure ConcatServlet behaves well if it's case insensitive.
|
public static Stream<Arguments> webInfTestExamples()
|
||||||
uri = contextPath + concatPath + "?/trick/../web-inf/one.js";
|
{
|
||||||
request =
|
return Stream.of(
|
||||||
|
// Cannot access WEB-INF.
|
||||||
|
Arguments.of("?/WEB-INF/", "HTTP/1.1 404 "),
|
||||||
|
Arguments.of("?/WEB-INF/one.js", "HTTP/1.1 404 "),
|
||||||
|
|
||||||
|
// Having a path segment and then ".." triggers a special case that the ConcatServlet must detect and avoid.
|
||||||
|
Arguments.of("?/trick/../WEB-INF/one.js", "HTTP/1.1 404 "),
|
||||||
|
|
||||||
|
// Make sure ConcatServlet behaves well if it's case insensitive.
|
||||||
|
Arguments.of("?/trick/../web-inf/one.js", "HTTP/1.1 404 "),
|
||||||
|
|
||||||
|
// Make sure ConcatServlet behaves well if encoded.
|
||||||
|
Arguments.of("?/trick/..%2FWEB-INF%2Fone.js", "HTTP/1.1 404 "),
|
||||||
|
Arguments.of("?/%2557EB-INF/one.js", "HTTP/1.1 500 "),
|
||||||
|
Arguments.of("?/js/%252e%252e/WEB-INF/one.js", "HTTP/1.1 500 ")
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
@ParameterizedTest
|
||||||
|
@MethodSource("webInfTestExamples")
|
||||||
|
public void testWEBINFResourceIsNotServed(String querystring, String expectedStatus) throws Exception
|
||||||
|
{
|
||||||
|
File directoryFile = MavenTestingUtils.getTargetTestingDir();
|
||||||
|
Path directoryPath = directoryFile.toPath();
|
||||||
|
Path hiddenDirectory = directoryPath.resolve("WEB-INF");
|
||||||
|
Files.createDirectories(hiddenDirectory);
|
||||||
|
Path hiddenResource = hiddenDirectory.resolve("one.js");
|
||||||
|
try (OutputStream output = Files.newOutputStream(hiddenResource))
|
||||||
|
{
|
||||||
|
output.write("function() {}".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
String contextPath = "";
|
||||||
|
WebAppContext context = new WebAppContext(server, directoryPath.toString(), contextPath);
|
||||||
|
server.setHandler(context);
|
||||||
|
String concatPath = "/concat";
|
||||||
|
context.addServlet(ConcatServlet.class, concatPath);
|
||||||
|
server.start();
|
||||||
|
|
||||||
|
// Verify that I can get the file programmatically, as required by the spec.
|
||||||
|
assertNotNull(context.getServletContext().getResource("/WEB-INF/one.js"));
|
||||||
|
|
||||||
|
String uri = contextPath + concatPath + querystring;
|
||||||
|
String request =
|
||||||
"GET " + uri + " HTTP/1.1\r\n" +
|
"GET " + uri + " HTTP/1.1\r\n" +
|
||||||
"Host: localhost\r\n" +
|
"Host: localhost\r\n" +
|
||||||
"Connection: close\r\n" +
|
"Connection: close\r\n" +
|
||||||
"\r\n";
|
"\r\n";
|
||||||
response = connector.getResponse(request);
|
String response = connector.getResponse(request);
|
||||||
assertTrue(response.startsWith("HTTP/1.1 404 "));
|
assertThat(response, startsWith(expectedStatus));
|
||||||
|
|
||||||
// Make sure ConcatServlet behaves well if encoded.
|
|
||||||
uri = contextPath + concatPath + "?/trick/..%2FWEB-INF%2Fone.js";
|
|
||||||
request =
|
|
||||||
"GET " + uri + " HTTP/1.1\r\n" +
|
|
||||||
"Host: localhost\r\n" +
|
|
||||||
"Connection: close\r\n" +
|
|
||||||
"\r\n";
|
|
||||||
response = connector.getResponse(request);
|
|
||||||
assertTrue(response.startsWith("HTTP/1.1 404 "));
|
|
||||||
|
|
||||||
// Make sure ConcatServlet cannot see file system files.
|
|
||||||
uri = contextPath + concatPath + "?/trick/../../" + directoryFile.getName();
|
|
||||||
request =
|
|
||||||
"GET " + uri + " HTTP/1.1\r\n" +
|
|
||||||
"Host: localhost\r\n" +
|
|
||||||
"Connection: close\r\n" +
|
|
||||||
"\r\n";
|
|
||||||
response = connector.getResponse(request);
|
|
||||||
assertTrue(response.startsWith("HTTP/1.1 404 "));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -0,0 +1,141 @@
|
||||||
|
//
|
||||||
|
// ========================================================================
|
||||||
|
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
|
||||||
|
//
|
||||||
|
// This program and the accompanying materials are made available under the
|
||||||
|
// terms of the Eclipse Public License v. 2.0 which is available at
|
||||||
|
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
|
||||||
|
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
|
||||||
|
//
|
||||||
|
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
|
||||||
|
// ========================================================================
|
||||||
|
//
|
||||||
|
|
||||||
|
package org.eclipse.jetty.servlets;
|
||||||
|
|
||||||
|
import java.io.OutputStream;
|
||||||
|
import java.nio.charset.StandardCharsets;
|
||||||
|
import java.nio.file.Files;
|
||||||
|
import java.nio.file.Path;
|
||||||
|
import java.util.EnumSet;
|
||||||
|
import java.util.stream.Stream;
|
||||||
|
import javax.servlet.DispatcherType;
|
||||||
|
|
||||||
|
import org.eclipse.jetty.http.UriCompliance;
|
||||||
|
import org.eclipse.jetty.server.HttpConnectionFactory;
|
||||||
|
import org.eclipse.jetty.server.LocalConnector;
|
||||||
|
import org.eclipse.jetty.server.Server;
|
||||||
|
import org.eclipse.jetty.servlet.FilterHolder;
|
||||||
|
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
|
||||||
|
import org.eclipse.jetty.webapp.WebAppContext;
|
||||||
|
import org.junit.jupiter.api.AfterEach;
|
||||||
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
|
import org.junit.jupiter.params.ParameterizedTest;
|
||||||
|
import org.junit.jupiter.params.provider.Arguments;
|
||||||
|
import org.junit.jupiter.params.provider.MethodSource;
|
||||||
|
|
||||||
|
import static org.hamcrest.MatcherAssert.assertThat;
|
||||||
|
import static org.hamcrest.Matchers.containsString;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||||
|
|
||||||
|
public class WelcomeFilterTest
|
||||||
|
{
|
||||||
|
private Server server;
|
||||||
|
private LocalConnector connector;
|
||||||
|
|
||||||
|
@BeforeEach
|
||||||
|
public void prepareServer() throws Exception
|
||||||
|
{
|
||||||
|
server = new Server();
|
||||||
|
connector = new LocalConnector(server);
|
||||||
|
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986);
|
||||||
|
server.addConnector(connector);
|
||||||
|
|
||||||
|
Path directoryPath = MavenTestingUtils.getTargetTestingDir().toPath();
|
||||||
|
Files.createDirectories(directoryPath);
|
||||||
|
Path welcomeResource = directoryPath.resolve("welcome.html");
|
||||||
|
try (OutputStream output = Files.newOutputStream(welcomeResource))
|
||||||
|
{
|
||||||
|
output.write("<h1>welcome page</h1>".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
Path otherResource = directoryPath.resolve("other.html");
|
||||||
|
try (OutputStream output = Files.newOutputStream(otherResource))
|
||||||
|
{
|
||||||
|
output.write("<h1>other resource</h1>".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
Path hiddenDirectory = directoryPath.resolve("WEB-INF");
|
||||||
|
Files.createDirectories(hiddenDirectory);
|
||||||
|
Path hiddenResource = hiddenDirectory.resolve("one.js");
|
||||||
|
try (OutputStream output = Files.newOutputStream(hiddenResource))
|
||||||
|
{
|
||||||
|
output.write("CONFIDENTIAL".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
Path hiddenWelcome = hiddenDirectory.resolve("index.html");
|
||||||
|
try (OutputStream output = Files.newOutputStream(hiddenWelcome))
|
||||||
|
{
|
||||||
|
output.write("CONFIDENTIAL".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
WebAppContext context = new WebAppContext(server, directoryPath.toString(), "/");
|
||||||
|
server.setHandler(context);
|
||||||
|
String concatPath = "/*";
|
||||||
|
|
||||||
|
FilterHolder filterHolder = new FilterHolder(new WelcomeFilter());
|
||||||
|
filterHolder.setInitParameter("welcome", "welcome.html");
|
||||||
|
context.addFilter(filterHolder, concatPath, EnumSet.of(DispatcherType.REQUEST));
|
||||||
|
server.start();
|
||||||
|
|
||||||
|
// Verify that I can get the file programmatically, as required by the spec.
|
||||||
|
assertNotNull(context.getServletContext().getResource("/WEB-INF/one.js"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@AfterEach
|
||||||
|
public void destroy() throws Exception
|
||||||
|
{
|
||||||
|
if (server != null)
|
||||||
|
server.stop();
|
||||||
|
}
|
||||||
|
|
||||||
|
public static Stream<Arguments> argumentsStream()
|
||||||
|
{
|
||||||
|
return Stream.of(
|
||||||
|
// Normal requests for the directory are redirected to the welcome page.
|
||||||
|
Arguments.of("/", new String[]{"HTTP/1.1 200 ", "<h1>welcome page</h1>"}),
|
||||||
|
|
||||||
|
// Try a normal resource (will bypass the filter).
|
||||||
|
Arguments.of("/other.html", new String[]{"HTTP/1.1 200 ", "<h1>other resource</h1>"}),
|
||||||
|
|
||||||
|
// Cannot access files in WEB-INF.
|
||||||
|
Arguments.of("/WEB-INF/one.js", new String[]{"HTTP/1.1 404 "}),
|
||||||
|
|
||||||
|
// Cannot serve welcome from WEB-INF.
|
||||||
|
Arguments.of("/WEB-INF/", new String[]{"HTTP/1.1 404 "}),
|
||||||
|
|
||||||
|
// Try to trick the filter into serving a protected resource.
|
||||||
|
Arguments.of("/WEB-INF/one.js#/", new String[]{"HTTP/1.1 404 "}),
|
||||||
|
Arguments.of("/js/../WEB-INF/one.js#/", new String[]{"HTTP/1.1 404 "}),
|
||||||
|
|
||||||
|
// Test the URI is not double decoded in the dispatcher.
|
||||||
|
Arguments.of("/%2557EB-INF/one.js%23/", new String[]{"HTTP/1.1 404 "})
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
@ParameterizedTest
|
||||||
|
@MethodSource("argumentsStream")
|
||||||
|
public void testWelcomeFilter(String uri, String[] contains) throws Exception
|
||||||
|
{
|
||||||
|
String request =
|
||||||
|
"GET " + uri + " HTTP/1.1\r\n" +
|
||||||
|
"Host: localhost\r\n" +
|
||||||
|
"Connection: close\r\n" +
|
||||||
|
"\r\n";
|
||||||
|
String response = connector.getResponse(request);
|
||||||
|
for (String s : contains)
|
||||||
|
{
|
||||||
|
assertThat(response, containsString(s));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,140 @@
|
||||||
|
//
|
||||||
|
// ========================================================================
|
||||||
|
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
|
||||||
|
//
|
||||||
|
// This program and the accompanying materials are made available under the
|
||||||
|
// terms of the Eclipse Public License v. 2.0 which is available at
|
||||||
|
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
|
||||||
|
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
|
||||||
|
//
|
||||||
|
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
|
||||||
|
// ========================================================================
|
||||||
|
//
|
||||||
|
|
||||||
|
package org.eclipse.jetty.webapp;
|
||||||
|
|
||||||
|
import java.io.OutputStream;
|
||||||
|
import java.nio.charset.StandardCharsets;
|
||||||
|
import java.nio.file.Files;
|
||||||
|
import java.nio.file.Path;
|
||||||
|
import java.util.stream.Stream;
|
||||||
|
|
||||||
|
import org.eclipse.jetty.http.UriCompliance;
|
||||||
|
import org.eclipse.jetty.server.HttpConnectionFactory;
|
||||||
|
import org.eclipse.jetty.server.LocalConnector;
|
||||||
|
import org.eclipse.jetty.server.Server;
|
||||||
|
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
|
||||||
|
import org.eclipse.jetty.util.IO;
|
||||||
|
import org.junit.jupiter.api.AfterEach;
|
||||||
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
|
import org.junit.jupiter.params.ParameterizedTest;
|
||||||
|
import org.junit.jupiter.params.provider.Arguments;
|
||||||
|
import org.junit.jupiter.params.provider.MethodSource;
|
||||||
|
|
||||||
|
import static org.hamcrest.MatcherAssert.assertThat;
|
||||||
|
import static org.hamcrest.Matchers.containsString;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||||
|
|
||||||
|
public class WebAppDefaultServletTest
|
||||||
|
{
|
||||||
|
private Server server;
|
||||||
|
private LocalConnector connector;
|
||||||
|
|
||||||
|
@BeforeEach
|
||||||
|
public void prepareServer() throws Exception
|
||||||
|
{
|
||||||
|
server = new Server();
|
||||||
|
connector = new LocalConnector(server);
|
||||||
|
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986);
|
||||||
|
server.addConnector(connector);
|
||||||
|
|
||||||
|
Path directoryPath = MavenTestingUtils.getTargetTestingDir().toPath();
|
||||||
|
IO.delete(directoryPath.toFile());
|
||||||
|
Files.createDirectories(directoryPath);
|
||||||
|
Path welcomeResource = directoryPath.resolve("index.html");
|
||||||
|
try (OutputStream output = Files.newOutputStream(welcomeResource))
|
||||||
|
{
|
||||||
|
output.write("<h1>welcome page</h1>".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
Path otherResource = directoryPath.resolve("other.html");
|
||||||
|
try (OutputStream output = Files.newOutputStream(otherResource))
|
||||||
|
{
|
||||||
|
output.write("<h1>other resource</h1>".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
Path hiddenDirectory = directoryPath.resolve("WEB-INF");
|
||||||
|
Files.createDirectories(hiddenDirectory);
|
||||||
|
Path hiddenResource = hiddenDirectory.resolve("one.js");
|
||||||
|
try (OutputStream output = Files.newOutputStream(hiddenResource))
|
||||||
|
{
|
||||||
|
output.write("this is confidential".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create directory to trick resource service.
|
||||||
|
Path hackPath = directoryPath.resolve("%57EB-INF/one.js#/");
|
||||||
|
Files.createDirectories(hackPath);
|
||||||
|
try (OutputStream output = Files.newOutputStream(hackPath.resolve("index.html")))
|
||||||
|
{
|
||||||
|
output.write("this content does not matter".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
Path standardHashDir = directoryPath.resolve("welcome#");
|
||||||
|
Files.createDirectories(standardHashDir);
|
||||||
|
try (OutputStream output = Files.newOutputStream(standardHashDir.resolve("index.html")))
|
||||||
|
{
|
||||||
|
output.write("standard hash dir welcome".getBytes(StandardCharsets.UTF_8));
|
||||||
|
}
|
||||||
|
|
||||||
|
WebAppContext context = new WebAppContext(server, directoryPath.toString(), "/");
|
||||||
|
server.setHandler(context);
|
||||||
|
server.start();
|
||||||
|
|
||||||
|
// Verify that I can get the file programmatically, as required by the spec.
|
||||||
|
assertNotNull(context.getServletContext().getResource("/WEB-INF/one.js"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@AfterEach
|
||||||
|
public void destroy() throws Exception
|
||||||
|
{
|
||||||
|
if (server != null)
|
||||||
|
server.stop();
|
||||||
|
}
|
||||||
|
|
||||||
|
public static Stream<Arguments> argumentsStream()
|
||||||
|
{
|
||||||
|
return Stream.of(
|
||||||
|
Arguments.of("/WEB-INF/", new String[]{"HTTP/1.1 404 "}),
|
||||||
|
Arguments.of("/welcome%23/", new String[]{"HTTP/1.1 200 ", "standard hash dir welcome"}),
|
||||||
|
|
||||||
|
// Normal requests for the directory are redirected to the welcome page.
|
||||||
|
Arguments.of("/", new String[]{"HTTP/1.1 200 ", "<h1>welcome page</h1>"}),
|
||||||
|
|
||||||
|
// We can be served other resources.
|
||||||
|
Arguments.of("/other.html", new String[]{"HTTP/1.1 200 ", "<h1>other resource</h1>"}),
|
||||||
|
|
||||||
|
// The ContextHandler will filter these ones out as as WEB-INF is a protected target.
|
||||||
|
Arguments.of("/WEB-INF/one.js#/", new String[]{"HTTP/1.1 404 "}),
|
||||||
|
Arguments.of("/js/../WEB-INF/one.js#/", new String[]{"HTTP/1.1 404 "}),
|
||||||
|
|
||||||
|
// Test the URI is not double decoded by the dispatcher that serves the welcome file (we get index.html not one.js).
|
||||||
|
Arguments.of("/%2557EB-INF/one.js%23/", new String[]{"HTTP/1.1 200 ", "this content does not matter"})
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
@ParameterizedTest
|
||||||
|
@MethodSource("argumentsStream")
|
||||||
|
public void testResourceService(String uri, String[] contains) throws Exception
|
||||||
|
{
|
||||||
|
String request =
|
||||||
|
"GET " + uri + " HTTP/1.1\r\n" +
|
||||||
|
"Host: localhost\r\n" +
|
||||||
|
"Connection: close\r\n" +
|
||||||
|
"\r\n";
|
||||||
|
String response = connector.getResponse(request);
|
||||||
|
for (String s : contains)
|
||||||
|
{
|
||||||
|
assertThat(response, containsString(s));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue