Skip to content

Commit

Permalink
Cherry-pick of Improvements to PathSpec for Jetty 10.0.x (#8136)
Browse files Browse the repository at this point in the history
* Cherry-pick of Improvements to PathSpec.
* From commit: 5b4d1dd
* Fixing ConstraintSecurityHandler usage of PathMappings
* Fixing bad INCLUDE logic from cherry-pick in ServletHandler.doScope()
* Cleanup of non ServletPathSpec behaviors in ServletPathMapping class
* Skip optional group name/info lookup if regex fails.
* Prevent NPE on static servletPathMappings
* Update WebSocketMappings to use new PathMappings.getMatched(String)

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
joakime committed Oct 6, 2022
1 parent 351fe53 commit 25dd6d0
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 74 deletions.
Expand Up @@ -178,13 +178,14 @@ public MatchedResource<E> getMatched(String path)
{
MappedResource<E> candidate = _exactMap.getBest(path, 0, i--);
if (candidate == null)
continue;
break;

matchedPath = candidate.getPathSpec().matched(path);
if (matchedPath != null)
{
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
}
i--;
}
// If we reached here, there's NO optimized EXACT Match possible, skip simple match below
skipRestOfGroup = true;
Expand All @@ -201,11 +202,12 @@ public MatchedResource<E> getMatched(String path)
{
MappedResource<E> candidate = _prefixMap.getBest(path, 0, i--);
if (candidate == null)
continue;
break;

matchedPath = candidate.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
i--;
}
// If we reached here, there's NO optimized PREFIX Match possible, skip simple match below
skipRestOfGroup = true;
Expand All @@ -227,7 +229,7 @@ public MatchedResource<E> getMatched(String path)
{
MappedResource<E> candidate = _suffixMap.get(path, i + 1, path.length() - i - 1);
if (candidate == null)
continue;
break;

matchedPath = candidate.getPathSpec().matched(path);
if (matchedPath != null)
Expand Down Expand Up @@ -259,6 +261,15 @@ public Iterator<MappedResource<E>> iterator()
return _mappings.iterator();
}

/**
* @deprecated use {@link PathSpec#from(String)} instead
*/
@Deprecated
public static PathSpec asPathSpec(String pathSpecString)
{
return PathSpec.from(pathSpecString);
}

public E get(PathSpec spec)
{
return _mappings.stream()
Expand Down
Expand Up @@ -125,9 +125,8 @@ public RegexPathSpec(String regex)
declaration = regex;
int specLength = declaration.length();
// build up a simple signature we can use to identify the grouping
boolean inCharacterClass = false;
boolean inTextList = false;
boolean inQuantifier = false;
boolean inCaptureGroup = false;
StringBuilder signature = new StringBuilder();

int pathDepth = 0;
Expand All @@ -140,6 +139,8 @@ public RegexPathSpec(String regex)
case '^': // ignore anchors
case '$': // ignore anchors
case '\'': // ignore escaping
case '(': // ignore grouping
case ')': // ignore grouping
break;
case '+': // single char quantifier
case '?': // single char quantifier
Expand All @@ -148,32 +149,25 @@ public RegexPathSpec(String regex)
case '.': // any char token
signature.append('g'); // glob
break;
case '(': // in regex capture group
inCaptureGroup = true;
break;
case ')':
inCaptureGroup = false;
signature.append('g');
break;
case '{': // in regex quantifier
case '{':
inQuantifier = true;
break;
case '}':
inQuantifier = false;
break;
case '[': // in regex character class
inCharacterClass = true;
case '[':
inTextList = true;
break;
case ']':
inCharacterClass = false;
inTextList = false;
signature.append('g'); // glob
break;
case '/':
if (!inCharacterClass && !inQuantifier && !inCaptureGroup)
if (!inTextList && !inQuantifier)
pathDepth++;
break;
default:
if (!inCharacterClass && !inQuantifier && !inCaptureGroup && Character.isLetterOrDigit(c))
if (!inTextList && !inQuantifier && Character.isLetterOrDigit(c))
{
if (last == '\\') // escaped
{
Expand Down Expand Up @@ -325,6 +319,101 @@ public MatchedPath matched(String path)
return null;
}

private class RegexMatchedPath implements MatchedPath
{
private final RegexPathSpec pathSpec;
private final String path;
private final Matcher matcher;

public RegexMatchedPath(RegexPathSpec regexPathSpec, String path, Matcher matcher)
{
this.pathSpec = regexPathSpec;
this.path = path;
this.matcher = matcher;
}

@Override
public String getPathMatch()
{
try
{
String p = matcher.group("name");
if (p != null)
{
return p;
}
}
catch (IllegalArgumentException ignore)
{
// ignore if group name not found.
}

if (pathSpec.getGroup() == PathSpecGroup.PREFIX_GLOB && matcher.groupCount() >= 1)
{
int idx = matcher.start(1);
if (idx > 0)
{
if (this.path.charAt(idx - 1) == '/')
idx--;
return this.path.substring(0, idx);
}
}

// default is the full path
return this.path;
}

@Override
public String getPathInfo()
{
try
{
String p = matcher.group("info");
if (p != null)
{
return p;
}
}
catch (IllegalArgumentException ignore)
{
// ignore if group info not found.
}

// Path Info only valid for PREFIX_GLOB
if (pathSpec.getGroup() == PathSpecGroup.PREFIX_GLOB && matcher.groupCount() >= 1)
{
String pathInfo = matcher.group(1);
if ("".equals(pathInfo))
return "/";
else
return pathInfo;
}

// default is null
return null;
}

@Override
public String toString()
{
return getClass().getSimpleName() + "[" +
"pathSpec=" + pathSpec +
", path=\"" + path + "\"" +
", matcher=" + matcher +
']';
}

@Override
public MatchedPath matched(String path)
{
Matcher matcher = getMatcher(path);
if (matcher.matches())
{
return new RegexMatchedPath(this, path, matcher);
}
return null;
}

private class RegexMatchedPath implements MatchedPath
{
private final RegexPathSpec pathSpec;
Expand Down
Expand Up @@ -28,6 +28,9 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class RegexPathSpecTest
{
Expand Down Expand Up @@ -201,8 +204,8 @@ public void testSuffixSpecTraditional()
assertNotMatches(spec, "/aa/bb");
assertNotMatches(spec, "/aa/bb.do/more");

assertThat(spec.getPathMatch("/a/b/c.do"), equalTo(""));
assertThat(spec.getPathInfo("/a/b/c.do"), equalTo("/a/b/c.do"));
assertThat(spec.getPathMatch("/a/b/c.do"), equalTo("/a/b/c.do"));
assertThat(spec.getPathInfo("/a/b/c.do"), nullValue());
}

/**
Expand Down
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

0 comments on commit 25dd6d0

Please sign in to comment.