Fixing #9517 - bad url-pattern prefix match behavior (#9518)

* Fixing #9517 - bad url-pattern prefix match behavior

Fixed regression where `/foo/*` was incorrectly matching `/foobar`.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: gregw <gregw@webtide.com>
Co-authored-by: gregw <gregw@webtide.com>
This commit is contained in:
Joakim Erdfelt 2023-03-20 11:53:40 -05:00 committed by GitHub
parent fe505766fd
commit 17aa0c5ab9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 72 additions and 15 deletions

View File

@ -43,6 +43,8 @@ import org.slf4j.LoggerFactory;
public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
{
private static final Logger LOG = LoggerFactory.getLogger(PathMappings.class);
// In prefix matches, this is the length ("/*".length() + 1) - used for the best prefix match loop
private static final int PREFIX_TAIL_LEN = 3;
private final Set<MappedResource<E>> _mappings = new TreeSet<>(Comparator.comparing(MappedResource::getPathSpec));
/**
@ -205,11 +207,14 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
// Try a prefix match
MappedResource<E> prefix = _prefixMap.getBest(path);
if (prefix != null)
while (prefix != null)
{
MatchedPath matchedPath = prefix.getPathSpec().matched(path);
PathSpec pathSpec = prefix.getPathSpec();
MatchedPath matchedPath = pathSpec.matched(path);
if (matchedPath != null)
return new MatchedResource<>(prefix.getResource(), prefix.getPathSpec(), matchedPath);
return new MatchedResource<>(prefix.getResource(), pathSpec, matchedPath);
int specLength = pathSpec.getSpecLength();
prefix = specLength > PREFIX_TAIL_LEN ? _prefixMap.getBest(path, 0, specLength - PREFIX_TAIL_LEN) : null;
}
// Try a suffix match
@ -223,13 +228,13 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
// Loop 3: "foo"
while ((i = path.indexOf('.', i + 1)) > 0)
{
prefix = _suffixMap.get(path, i + 1, path.length() - i - 1);
if (prefix == null)
MappedResource<E> suffix = _suffixMap.get(path, i + 1, path.length() - i - 1);
if (suffix == null)
continue;
MatchedPath matchedPath = prefix.getPathSpec().matched(path);
MatchedPath matchedPath = suffix.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(prefix.getResource(), prefix.getPathSpec(), matchedPath);
return new MatchedResource<>(suffix.getResource(), suffix.getPathSpec(), matchedPath);
}
}
@ -286,12 +291,15 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
{
if (_optimizedPrefix)
{
MappedResource<E> candidate = _prefixMap.getBest(path);
if (candidate != null)
MappedResource<E> prefix = _prefixMap.getBest(path);
while (prefix != null)
{
matchedPath = candidate.getPathSpec().matched(path);
PathSpec pathSpec = prefix.getPathSpec();
matchedPath = pathSpec.matched(path);
if (matchedPath != null)
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
return new MatchedResource<>(prefix.getResource(), pathSpec, matchedPath);
int specLength = pathSpec.getSpecLength();
prefix = specLength > PREFIX_TAIL_LEN ? _prefixMap.getBest(path, 0, specLength - PREFIX_TAIL_LEN) : null;
}
// If we reached here, there's NO optimized PREFIX Match possible, skip simple match below
@ -312,13 +320,13 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
// Loop 3: "foo"
while ((i = path.indexOf('.', i + 1)) > 0)
{
MappedResource<E> candidate = _suffixMap.get(path, i + 1, path.length() - i - 1);
if (candidate == null)
MappedResource<E> suffix = _suffixMap.get(path, i + 1, path.length() - i - 1);
if (suffix == null)
continue;
matchedPath = candidate.getPathSpec().matched(path);
matchedPath = suffix.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
return new MatchedResource<>(suffix.getResource(), suffix.getPathSpec(), matchedPath);
}
// If we reached here, there's NO optimized SUFFIX Match possible, skip simple match below
skipRestOfGroup = true;

View File

@ -92,6 +92,55 @@ public class PathMappingsTest
assertMatch(p, "/", "any");
}
/**
* Test the match order rules imposed by the Servlet API (any vs specific sub-dir)
*/
@Test
public void testServletMatchPrefix()
{
PathMappings<String> p = new PathMappings<>();
p.put(new ServletPathSpec("/*"), "any");
p.put(new ServletPathSpec("/foo/*"), "foo");
p.put(new ServletPathSpec("/food/*"), "food");
p.put(new ServletPathSpec("/a/*"), "a");
p.put(new ServletPathSpec("/a/b/*"), "ab");
assertMatch(p, "/abs/path", "any");
assertMatch(p, "/abs/foo/bar", "any");
assertMatch(p, "/foo/bar", "foo");
assertMatch(p, "/", "any");
assertMatch(p, "/foo", "foo");
assertMatch(p, "/fo", "any");
assertMatch(p, "/foobar", "any");
assertMatch(p, "/foob", "any");
assertMatch(p, "/food", "food");
assertMatch(p, "/food/zed", "food");
assertMatch(p, "/foodie", "any");
assertMatch(p, "/a/bc", "a");
assertMatch(p, "/a/b/c", "ab");
assertMatch(p, "/a/", "a");
assertMatch(p, "/a", "a");
// Try now with order important
p.put(new RegexPathSpec("/other.*/"), "other");
assertMatch(p, "/abs/path", "any");
assertMatch(p, "/abs/foo/bar", "any");
assertMatch(p, "/foo/bar", "foo");
assertMatch(p, "/", "any");
assertMatch(p, "/foo", "foo");
assertMatch(p, "/fo", "any");
assertMatch(p, "/foobar", "any");
assertMatch(p, "/foob", "any");
assertMatch(p, "/food", "food");
assertMatch(p, "/food/zed", "food");
assertMatch(p, "/foodie", "any");
assertMatch(p, "/a/bc", "a");
assertMatch(p, "/a/b/c", "ab");
assertMatch(p, "/a/", "a");
assertMatch(p, "/a", "a");
}
/**
* Test the match order rules with a mixed Servlet and URI Template path specs
*