From 6c62157865f124c83c5b8e7278af8acf140883f2 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Wed, 3 Jun 2020 14:26:35 +0200 Subject: [PATCH] Signed-off-by: Ludovic Orban --- .../jetty/http/pathmap/PathMappings.java | 11 +- .../eclipse/jetty/http/pathmap/PathSpec.java | 2 + .../jetty/http/pathmap/RegexPathSpec.java | 18 ++ .../jetty/http/pathmap/ServletPathSpec.java | 19 +++ .../http/pathmap/UriTemplatePathSpec.java | 17 ++ .../jetty/http/pathmap/PathMappingsTest.java | 159 ++++++++++++++++++ .../jetty/http/pathmap/RegexPathSpecTest.java | 12 ++ .../http/pathmap/ServletPathSpecTest.java | 15 ++ .../http/pathmap/UriTemplatePathSpecTest.java | 13 ++ 9 files changed, 263 insertions(+), 3 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java index bf59e908b03..6901ff69c3e 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java @@ -261,16 +261,21 @@ public class PathMappings implements Iterable>, 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; } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java index 8fb61043dc5..0221454294f 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java @@ -20,6 +20,8 @@ package org.eclipse.jetty.http.pathmap; /** * A path specification is a URI path template that can be matched against. + *

+ * Implementors must override {@link Object#equals(Object)} and {@link Object#hashCode()}. */ public interface PathSpec extends Comparable { diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java index 00dfaecedf0..9df701ca3e4 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.http.pathmap; +import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -193,4 +194,21 @@ public class RegexPathSpec implements PathSpec return getMatcher(path).matches(); } } + + @Override + public boolean equals(Object o) + { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + RegexPathSpec that = (RegexPathSpec)o; + return compareTo(that) == 0; + } + + @Override + public int hashCode() + { + return Objects.hash(_declaration); + } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java index eae37987e75..fab3224f896 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.http.pathmap; +import java.util.Objects; + import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -291,4 +293,21 @@ public class ServletPathSpec implements PathSpec return false; } } + + @Override + public boolean equals(Object o) + { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + ServletPathSpec that = (ServletPathSpec)o; + return compareTo(that) == 0; + } + + @Override + public int hashCode() + { + return Objects.hash(_declaration); + } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java index 1a0d6bae586..3757b1419a7 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java @@ -421,4 +421,21 @@ public class UriTemplatePathSpec implements PathSpec { return _variables; } + + @Override + public boolean equals(Object o) + { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + UriTemplatePathSpec that = (UriTemplatePathSpec)o; + return compareTo(that) == 0; + } + + @Override + public int hashCode() + { + return Objects.hash(_declaration); + } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java index 4a1fd7e037c..593128a2263 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java @@ -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 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 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 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 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 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 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 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)); + } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java index 0659f2fe37b..8b43b5f5a13 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java @@ -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")))); + } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java index ec52f2c0456..9ba1c7f6a33 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java @@ -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")))); + } + } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpecTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpecTest.java index 144597165d9..c7e3006a6cd 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpecTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpecTest.java @@ -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")))); + } }