Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4935 from lorban/jetty-9.4.x-4877-fix-PathMappings-get 

#4877 fix PathSpec long-standing bugs and shortcomings
This commit is contained in:
Joakim Erdfelt 2020-06-03 11:46:38 -05:00 committed by GitHub
parent 5d328ba928 e43c98f08d
commit 2834f93447
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 279 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java Normal file
View File

@ -0,0 +1,66 @@
//
// ========================================================================
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.http.pathmap;
import java.util.Objects;
public abstract class AbstractPathSpec implements PathSpec
{
@Override
public int compareTo(PathSpec other)
{
// Grouping (increasing)
int diff = getGroup().ordinal() - other.getGroup().ordinal();
if (diff != 0)
return diff;
// Spec Length (decreasing)
diff = other.getSpecLength() - getSpecLength();
if (diff != 0)
return diff;
// Path Spec Name (alphabetical)
return getDeclaration().compareTo(other.getDeclaration());
}
@Override
public final boolean equals(Object obj)
{
if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
return compareTo((AbstractPathSpec)obj) == 0;
}
@Override
public final int hashCode()
{
return Objects.hash(getDeclaration());
}
@Override
public String toString()
{
return String.format("%s@%s{%s}", getClass().getSimpleName(), Integer.toHexString(hashCode()), getDeclaration());
}
}

11
jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java
View File

@ -261,16 +261,21 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
@SuppressWarnings("incomplete-switch")
public boolean remove(PathSpec pathSpec)
{
String prefix = pathSpec.getPrefix();
String suffix = pathSpec.getSuffix();
switch (pathSpec.getGroup())
{
case EXACT:
_exactMap.remove(pathSpec.getPrefix());
if (prefix != null)
_exactMap.remove(prefix);
break;
case PREFIX_GLOB:
_prefixMap.remove(pathSpec.getPrefix());
if (prefix != null)
_prefixMap.remove(prefix);
break;
case SUFFIX_GLOB:
_suffixMap.remove(pathSpec.getSuffix());
if (suffix != null)
_suffixMap.remove(suffix);
break;
}

18
jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java
View File

@ -20,6 +20,8 @@ package org.eclipse.jetty.http.pathmap;
/**
* A path specification is a URI path template that can be matched against.
* <p>
* Implementors <i>must</i> override {@link Object#equals(Object)} and {@link Object#hashCode()}.
*/
public interface PathSpec extends Comparable<PathSpec>
{
@ -90,20 +92,4 @@ public interface PathSpec extends Comparable<PathSpec>
* @return true if the path matches this path spec, false otherwise
*/
boolean matches(String path);
default int compareTo(PathSpec other)
{
// Grouping (increasing)
int diff = getGroup().ordinal() - other.getGroup().ordinal();
if (diff != 0)
return diff;
// Spec Length (decreasing)
diff = other.getSpecLength() - getSpecLength();
if (diff != 0)
return diff;
// Path Spec Name (alphabetical)
return getDeclaration().compareTo(other.getDeclaration());
}
}

2
jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java
View File

@ -21,7 +21,7 @@ package org.eclipse.jetty.http.pathmap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
public class RegexPathSpec implements PathSpec
public class RegexPathSpec extends AbstractPathSpec
{
private final String _declaration;
private final PathSpecGroup _group;

2
jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java
View File

@ -22,7 +22,7 @@ import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
public class ServletPathSpec implements PathSpec
public class ServletPathSpec extends AbstractPathSpec
{
private static final Logger LOG = Log.getLogger(ServletPathSpec.class);

4
jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java
View File

@ -38,7 +38,7 @@ import org.eclipse.jetty.util.log.Logger;
*
* @see <a href="https://tools.ietf.org/html/rfc6570">URI Templates (Level 1)</a>
*/
public class UriTemplatePathSpec implements PathSpec
public class UriTemplatePathSpec extends AbstractPathSpec
{
private static final Logger LOG = Log.getLogger(UriTemplatePathSpec.class);
@ -287,7 +287,7 @@ public class UriTemplatePathSpec implements PathSpec
}
else
{
return PathSpec.super.compareTo(other);
return super.compareTo(other);
}
}

159
jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java
View File

@ -22,8 +22,11 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -302,4 +305,160 @@ public class PathMappingsTest
new ServletPathSpec(str);
});
}
@Test
void testPutRejectsDuplicates()
{
PathMappings<String> p = new PathMappings<>();
assertThat(p.put(new UriTemplatePathSpec("/a/{var1}/c"), "resourceA"), is(true));
assertThat(p.put(new UriTemplatePathSpec("/a/{var2}/c"), "resourceAA"), is(false));
assertThat(p.put(new UriTemplatePathSpec("/a/b/c"), "resourceB"), is(true));
assertThat(p.put(new UriTemplatePathSpec("/a/b/c"), "resourceBB"), is(false));
assertThat(p.put(new ServletPathSpec("/a/b/c"), "resourceBB"), is(false));
assertThat(p.put(new RegexPathSpec("/a/b/c"), "resourceBB"), is(false));
assertThat(p.put(new ServletPathSpec("/*"), "resourceC"), is(true));
assertThat(p.put(new RegexPathSpec("/(.*)"), "resourceCC"), is(true));
}
@Test
void testGetUriTemplatePathSpec()
{
PathMappings<String> p = new PathMappings<>();
p.put(new UriTemplatePathSpec("/a/{var1}/c"), "resourceA");
p.put(new UriTemplatePathSpec("/a/b/c"), "resourceB");
assertThat(p.get(new UriTemplatePathSpec("/a/{var1}/c")), equalTo("resourceA"));
assertThat(p.get(new UriTemplatePathSpec("/a/{foo}/c")), equalTo("resourceA"));
assertThat(p.get(new UriTemplatePathSpec("/a/b/c")), equalTo("resourceB"));
assertThat(p.get(new UriTemplatePathSpec("/a/d/c")), nullValue());
assertThat(p.get(new RegexPathSpec("/a/b/c")), nullValue());
}
@Test
void testGetRegexPathSpec()
{
PathMappings<String> p = new PathMappings<>();
p.put(new RegexPathSpec("/a/b/c"), "resourceA");
p.put(new RegexPathSpec("/(.*)/b/c"), "resourceB");
p.put(new RegexPathSpec("/a/(.*)/c"), "resourceC");
p.put(new RegexPathSpec("/a/b/(.*)"), "resourceD");
assertThat(p.get(new RegexPathSpec("/a/(.*)/c")), equalTo("resourceC"));
assertThat(p.get(new RegexPathSpec("/a/b/c")), equalTo("resourceA"));
assertThat(p.get(new RegexPathSpec("/(.*)/b/c")), equalTo("resourceB"));
assertThat(p.get(new RegexPathSpec("/a/b/(.*)")), equalTo("resourceD"));
assertThat(p.get(new RegexPathSpec("/a/d/c")), nullValue());
assertThat(p.get(new ServletPathSpec("/a/b/c")), nullValue());
}
@Test
void testGetServletPathSpec()
{
PathMappings<String> p = new PathMappings<>();
p.put(new ServletPathSpec("/"), "resourceA");
p.put(new ServletPathSpec("/*"), "resourceB");
p.put(new ServletPathSpec("/a/*"), "resourceC");
p.put(new ServletPathSpec("*.do"), "resourceD");
assertThat(p.get(new ServletPathSpec("/")), equalTo("resourceA"));
assertThat(p.get(new ServletPathSpec("/*")), equalTo("resourceB"));
assertThat(p.get(new ServletPathSpec("/a/*")), equalTo("resourceC"));
assertThat(p.get(new ServletPathSpec("*.do")), equalTo("resourceD"));
assertThat(p.get(new ServletPathSpec("*.gz")), nullValue());
assertThat(p.get(new ServletPathSpec("/a/b/*")), nullValue());
assertThat(p.get(new ServletPathSpec("/a/d/c")), nullValue());
assertThat(p.get(new RegexPathSpec("/a/b/c")), nullValue());
}
@Test
void testRemoveUriTemplatePathSpec()
{
PathMappings<String> p = new PathMappings<>();
p.put(new UriTemplatePathSpec("/a/{var1}/c"), "resourceA");
assertThat(p.remove(new UriTemplatePathSpec("/a/{var1}/c")), is(true));
p.put(new UriTemplatePathSpec("/a/{var1}/c"), "resourceA");
assertThat(p.remove(new UriTemplatePathSpec("/a/b/c")), is(false));
assertThat(p.remove(new UriTemplatePathSpec("/a/{b}/c")), is(true));
assertThat(p.remove(new UriTemplatePathSpec("/a/{b}/c")), is(false));
p.put(new UriTemplatePathSpec("/{var1}/b/c"), "resourceA");
assertThat(p.remove(new UriTemplatePathSpec("/a/b/c")), is(false));
assertThat(p.remove(new UriTemplatePathSpec("/{a}/b/c")), is(true));
assertThat(p.remove(new UriTemplatePathSpec("/{a}/b/c")), is(false));
p.put(new UriTemplatePathSpec("/a/b/{var1}"), "resourceA");
assertThat(p.remove(new UriTemplatePathSpec("/a/b/c")), is(false));
assertThat(p.remove(new UriTemplatePathSpec("/a/b/{c}")), is(true));
assertThat(p.remove(new UriTemplatePathSpec("/a/b/{c}")), is(false));
p.put(new UriTemplatePathSpec("/{var1}/{var2}/{var3}"), "resourceA");
assertThat(p.remove(new UriTemplatePathSpec("/a/b/c")), is(false));
assertThat(p.remove(new UriTemplatePathSpec("/{a}/{b}/{c}")), is(true));
assertThat(p.remove(new UriTemplatePathSpec("/{a}/{b}/{c}")), is(false));
}
@Test
void testRemoveRegexPathSpec()
{
PathMappings<String> p = new PathMappings<>();
p.put(new RegexPathSpec("/a/(.*)/c"), "resourceA");
assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(false));
assertThat(p.remove(new RegexPathSpec("/a/(.*)/c")), is(true));
assertThat(p.remove(new RegexPathSpec("/a/(.*)/c")), is(false));
p.put(new RegexPathSpec("/(.*)/b/c"), "resourceA");
assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(false));
assertThat(p.remove(new RegexPathSpec("/(.*)/b/c")), is(true));
assertThat(p.remove(new RegexPathSpec("/(.*)/b/c")), is(false));
p.put(new RegexPathSpec("/a/b/(.*)"), "resourceA");
assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(false));
assertThat(p.remove(new RegexPathSpec("/a/b/(.*)")), is(true));
assertThat(p.remove(new RegexPathSpec("/a/b/(.*)")), is(false));
p.put(new RegexPathSpec("/a/b/c"), "resourceA");
assertThat(p.remove(new RegexPathSpec("/a/b/d")), is(false));
assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(true));
assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(false));
}
@Test
void testRemoveServletPathSpec()
{
PathMappings<String> p = new PathMappings<>();
p.put(new ServletPathSpec("/a/*"), "resourceA");
assertThat(p.remove(new ServletPathSpec("/a/b")), is(false));
assertThat(p.remove(new ServletPathSpec("/a/*")), is(true));
assertThat(p.remove(new ServletPathSpec("/a/*")), is(false));
p.put(new ServletPathSpec("/a/b/*"), "resourceA");
assertThat(p.remove(new ServletPathSpec("/a/b/c")), is(false));
assertThat(p.remove(new ServletPathSpec("/a/b/*")), is(true));
assertThat(p.remove(new ServletPathSpec("/a/b/*")), is(false));
p.put(new ServletPathSpec("*.do"), "resourceA");
assertThat(p.remove(new ServletPathSpec("*.gz")), is(false));
assertThat(p.remove(new ServletPathSpec("*.do")), is(true));
assertThat(p.remove(new ServletPathSpec("*.do")), is(false));
p.put(new ServletPathSpec("/"), "resourceA");
assertThat(p.remove(new ServletPathSpec("/a")), is(false));
assertThat(p.remove(new ServletPathSpec("/")), is(true));
assertThat(p.remove(new ServletPathSpec("/")), is(false));
p.put(new ServletPathSpec(""), "resourceA");
assertThat(p.remove(new ServletPathSpec("/")), is(false));
assertThat(p.remove(new ServletPathSpec("")), is(true));
assertThat(p.remove(new ServletPathSpec("")), is(false));
p.put(new ServletPathSpec("/a/b/c"), "resourceA");
assertThat(p.remove(new ServletPathSpec("/a/b/d")), is(false));
assertThat(p.remove(new ServletPathSpec("/a/b/c")), is(true));
assertThat(p.remove(new ServletPathSpec("/a/b/c")), is(false));
}
}

12
jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java
View File

@ -21,7 +21,9 @@ package org.eclipse.jetty.http.pathmap;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class RegexPathSpecTest
@ -132,4 +134,14 @@ public class RegexPathSpecTest
assertNotMatches(spec, "/aa/bb");
assertNotMatches(spec, "/aa/bb.do/more");
}
@Test
void testEquals()
{
assertThat(new RegexPathSpec("^(.*).do$"), equalTo(new RegexPathSpec("^(.*).do$")));
assertThat(new RegexPathSpec("/foo"), equalTo(new RegexPathSpec("/foo")));
assertThat(new RegexPathSpec("^(.*).do$"), not(equalTo(new RegexPathSpec("^(.*).gz$"))));
assertThat(new RegexPathSpec("^(.*).do$"), not(equalTo(new RegexPathSpec("^.*.do$"))));
assertThat(new RegexPathSpec("/foo"), not(equalTo(new ServletPathSpec("/foo"))));
}
}

15
jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java
View File

@ -21,7 +21,9 @@ package org.eclipse.jetty.http.pathmap;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
@ -187,4 +189,17 @@ public class ServletPathSpecTest
assertEquals(null, spec.getPathInfo("/downloads/distribution.tar.gz"), "Spec.pathInfo");
}
@Test
void testEquals()
{
assertThat(new ServletPathSpec("*.gz"), equalTo(new ServletPathSpec("*.gz")));
assertThat(new ServletPathSpec("/foo"), equalTo(new ServletPathSpec("/foo")));
assertThat(new ServletPathSpec("/foo/bar"), equalTo(new ServletPathSpec("/foo/bar")));
assertThat(new ServletPathSpec("*.gz"), not(equalTo(new ServletPathSpec("*.do"))));
assertThat(new ServletPathSpec("/foo"), not(equalTo(new ServletPathSpec("/bar"))));
assertThat(new ServletPathSpec("/bar/foo"), not(equalTo(new ServletPathSpec("/foo/bar"))));
assertThat(new ServletPathSpec("/foo"), not(equalTo(new RegexPathSpec("/foo"))));
}
}

13
jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpecTest.java
View File

@ -23,7 +23,9 @@ import java.util.Map;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -281,4 +283,15 @@ public class UriTemplatePathSpecTest
assertThat("Spec.pathParams.size", mapped.size(), is(1));
assertEquals("a", mapped.get("var1"), "Spec.pathParams[var1]");
}
@Test
void testEquals()
{
assertThat(new UriTemplatePathSpec("/{var1}"), equalTo(new UriTemplatePathSpec("/{var1}")));
assertThat(new UriTemplatePathSpec("/{var1}"), equalTo(new UriTemplatePathSpec("/{var2}")));
assertThat(new UriTemplatePathSpec("/{var1}/{var2}"), equalTo(new UriTemplatePathSpec("/{var2}/{var1}")));
assertThat(new UriTemplatePathSpec("/{var1}"), not(equalTo(new UriTemplatePathSpec("/{var1}/{var2}"))));
assertThat(new UriTemplatePathSpec("/a/b/c"), not(equalTo(new UriTemplatePathSpec("/a/{var}/c"))));
assertThat(new UriTemplatePathSpec("/foo"), not(equalTo(new ServletPathSpec("/foo"))));
}
}