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

Jetty 12 - Cherry-Pick of Servlet PathMapping improvements around RegexPathSpec #8693

Merged
merged 8 commits into from
Oct 7, 2022
Merged

Jetty 12 - Cherry-Pick of Servlet PathMapping improvements around RegexPathSpec #8693

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
024a053
Issue #7748 - allow override of path mapping behavior in ServletConte…
gregw Mar 22, 2022
351fe53
Fix #7891 regex pathInfo (#7892)
gregw Apr 25, 2022
25dd6d0
Cherry-pick of Improvements to PathSpec for Jetty 10.0.x (#8136)
joakime Jun 8, 2022
3dd47d1
Cleanup post-cherry-pick of RegexPathSpec changes
joakime Oct 7, 2022
bada439
Missed ee9 changes
joakime Oct 7, 2022
dd3e3cf
Aligning changes between ee9 & ee10
joakime Oct 7, 2022
04d8c3f
Updates for test failures
joakime Oct 7, 2022
1ae87e0
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/jetty-12-…
joakime Oct 7, 2022
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
1 change: 1 addition & 0 deletions jetty-ee10/jetty-ee10-maven-plugin/src/test/resources/jetty-logging.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
#org.eclipse.jetty.LEVEL=DEBUG
#org.eclipse.jetty.server.LEVEL=DEBUG
#org.eclipse.jetty.http.LEVEL=DEBUG
#org.eclipse.jetty.http.pathmap.LEVEL=DEBUG
1 change: 1 addition & 0 deletions ...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,7 @@ protected void updateNameMappings()

protected PathSpec asPathSpec(String pathSpec)
{
// By default only allow servlet path specs
return new ServletPathSpec(pathSpec);
}

Expand Down
144 changes: 84 additions & 60 deletions ...0/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletPathMapping.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import jakarta.servlet.http.HttpServletMapping;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.MappingMatch;
import org.eclipse.jetty.http.pathmap.MatchedPath;
import org.eclipse.jetty.http.pathmap.PathSpec;
import org.eclipse.jetty.http.pathmap.ServletPathSpec;

Expand All @@ -38,77 +39,100 @@ public class ServletPathMapping implements HttpServletMapping
private final String _servletPath;
private final String _pathInfo;

public ServletPathMapping(PathSpec pathSpec, String servletName, String pathInContext)
public ServletPathMapping(PathSpec pathSpec, String servletName, String pathInContext, MatchedPath matchedPath)
{
_servletName = (servletName == null ? "" : servletName);
_pattern = pathSpec == null ? null : pathSpec.getDeclaration();

if (pathSpec instanceof ServletPathSpec && pathInContext != null)
{
switch (pathSpec.getGroup())
{
case ROOT:
_mappingMatch = MappingMatch.CONTEXT_ROOT;
_matchValue = "";
_servletPath = "";
_pathInfo = "/";
break;

case DEFAULT:
_mappingMatch = MappingMatch.DEFAULT;
_matchValue = "";
_servletPath = pathInContext;
_pathInfo = null;
break;

case EXACT:
_mappingMatch = MappingMatch.EXACT;
_matchValue = _pattern.startsWith("/") ? _pattern.substring(1) : _pattern;
_servletPath = _pattern;
_pathInfo = null;
break;

case PREFIX_GLOB:
_mappingMatch = MappingMatch.PATH;
_servletPath = pathSpec.getPrefix();
// TODO avoid the substring on the known servletPath!
_matchValue = _servletPath.startsWith("/") ? _servletPath.substring(1) : _servletPath;
_pathInfo = pathSpec.getPathInfo(pathInContext);
break;

case SUFFIX_GLOB:
_mappingMatch = MappingMatch.EXTENSION;
int dot = pathInContext.lastIndexOf('.');
_matchValue = pathInContext.substring(pathInContext.startsWith("/") ? 1 : 0, dot);
_servletPath = pathInContext;
_pathInfo = null;
break;

case MIDDLE_GLOB:
_mappingMatch = null;
_matchValue = "";
_servletPath = pathInContext;
_pathInfo = null;
break;

default:
throw new IllegalStateException();
}
}
else if (pathSpec != null)
if (pathSpec == null)
{
_pattern = null;
_mappingMatch = null;
_servletPath = pathSpec.getPathMatch(pathInContext);
_matchValue = _servletPath.startsWith("/") ? _servletPath.substring(1) : _servletPath;
_pathInfo = pathSpec.getPathInfo(pathInContext);
_matchValue = "";
_servletPath = pathInContext;
_pathInfo = null;
return;
}
else

if (pathInContext == null)
{
_pattern = pathSpec.getDeclaration();
_mappingMatch = null;
_matchValue = "";
_servletPath = pathInContext;
_servletPath = "";
_pathInfo = null;
return;
}

// Path Spec types that are not ServletPathSpec
if (!(pathSpec instanceof ServletPathSpec))
{
_pattern = pathSpec.getDeclaration();
_mappingMatch = null;
if (matchedPath != null)
{
_servletPath = matchedPath.getPathMatch();
_pathInfo = matchedPath.getPathInfo();
}
else
{
_servletPath = pathInContext;
_pathInfo = null;
}
_matchValue = _servletPath.substring(_servletPath.charAt(0) == '/' ? 1 : 0);
return;
}

// from here down is ServletPathSpec behavior
_pattern = pathSpec.getDeclaration();

switch (pathSpec.getGroup())
{
case ROOT:
_mappingMatch = MappingMatch.CONTEXT_ROOT;
_matchValue = "";
_servletPath = "";
_pathInfo = "/";
break;

case DEFAULT:
_mappingMatch = MappingMatch.DEFAULT;
_matchValue = "";
_servletPath = pathInContext;
_pathInfo = null;
break;

case EXACT:
_mappingMatch = MappingMatch.EXACT;
_matchValue = _pattern.startsWith("/") ? _pattern.substring(1) : _pattern;
_servletPath = _pattern;
_pathInfo = null;
break;

case PREFIX_GLOB:
_mappingMatch = MappingMatch.PATH;
_servletPath = pathSpec.getPrefix();
// TODO avoid the substring on the known servletPath!
_matchValue = _servletPath.startsWith("/") ? _servletPath.substring(1) : _servletPath;
_pathInfo = matchedPath != null ? matchedPath.getPathInfo() : null;
break;

case SUFFIX_GLOB:
_mappingMatch = MappingMatch.EXTENSION;
int dot = pathInContext.lastIndexOf('.');
_matchValue = pathInContext.substring(pathInContext.startsWith("/") ? 1 : 0, dot);
_servletPath = pathInContext;
_pathInfo = null;
break;

case MIDDLE_GLOB:
default:
throw new IllegalStateException("ServletPathSpec of type MIDDLE_GLOB");
}
}

public ServletPathMapping(PathSpec pathSpec, String servletName, String pathInContext)
{
this(pathSpec, servletName, pathInContext, null);
}

@Override
Expand Down
6 changes: 0 additions & 6 deletions ...y-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ServletPathMapping.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,6 @@ public ServletPathMapping(PathSpec pathSpec, String servletName, String pathInCo
break;

case MIDDLE_GLOB:
_mappingMatch = null;
_matchValue = "";
_servletPath = pathInContext;
_pathInfo = null;
break;

default:
throw new IllegalStateException("ServletPathSpec of type MIDDLE_GLOB");
}
Expand Down
10 changes: 5 additions & 5 deletions jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ public void testMultiPart() throws Exception
return s.count() == 2;
}
};

_server.stop();
_context.setContextPath("/foo");
_context.setResourceBase(".");
Expand Down Expand Up @@ -1686,11 +1686,11 @@ public void testCookieLeak() throws Exception
assertEquals("value", cookies[0]);
assertNull(cookies[1]);
}

/**
* Test that multiple requests on the same connection with different cookies
* do not bleed cookies.
*
*
* @throws Exception
*/
@Test
Expand Down Expand Up @@ -1733,7 +1733,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
response = HttpTester.parseResponse(lep.getResponse());
checkCookieResult(sessionId3, new String[] {sessionId1, sessionId2}, response.getContent());
}

@Test
public void testHashDOSKeys() throws Exception
{
Expand Down Expand Up @@ -2402,7 +2402,7 @@ public void clearAttributes()
{
}
}

private static void checkCookieResult(String containedCookie, String[] notContainedCookies, String response)
{
assertNotNull(containedCookie);
Expand Down
1 change: 1 addition & 0 deletions jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ServletHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1287,6 +1287,7 @@ protected void updateNameMappings()

protected PathSpec asPathSpec(String pathSpec)
{
// By default only allow servlet path specs
return new ServletPathSpec(pathSpec);
}

Expand Down
1 change: 0 additions & 1 deletion ...y-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/RegexServletTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.pathmap.PathMappings;
import org.eclipse.jetty.http.pathmap.PathSpec;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Server;
Expand Down