diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 6565a6e7a7b..b8dddded7ac 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -1805,7 +1805,7 @@ public class Request implements HttpServletRequest } else if (encoded.startsWith("/")) { - path = (encoded.length() == 1) ? "/" : URIUtil.canonicalPath(URIUtil.decodePath(encoded)); + path = (encoded.length() == 1) ? "/" : URIUtil.canonicalPath(uri.getDecodedPath()); } else if ("*".equals(encoded) || HttpMethod.CONNECT.is(getMethod())) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 7960b0fc91a..f2cfab1c4f7 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -41,6 +41,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.DispatcherType; import javax.servlet.Filter; import javax.servlet.FilterRegistration; @@ -230,10 +231,14 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu public enum Availability { - UNAVAILABLE, STARTING, AVAILABLE, SHUTDOWN, + STOPPED, // stopped and can't be made unavailable nor shutdown + STARTING, // starting inside of doStart. It may go to any of the next states. + AVAILABLE, // running normally + UNAVAILABLE, // Either a startup error or explicit call to setAvailable(false) + SHUTDOWN, // graceful shutdown } - private volatile Availability _availability = Availability.UNAVAILABLE; + private final AtomicReference _availability = new AtomicReference<>(Availability.STOPPED); public ContextHandler() { @@ -767,7 +772,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @ManagedAttribute("true for graceful shutdown, which allows existing requests to complete") public boolean isShutdown() { - return _availability == Availability.SHUTDOWN; + return _availability.get() == Availability.SHUTDOWN; } /** @@ -777,7 +782,24 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @Override public Future shutdown() { - _availability = isRunning() ? Availability.SHUTDOWN : Availability.UNAVAILABLE; + while (true) + { + Availability availability = _availability.get(); + switch (availability) + { + case STOPPED: + return new FutureCallback(new IllegalStateException(getState())); + case STARTING: + case AVAILABLE: + case UNAVAILABLE: + if (!_availability.compareAndSet(availability, Availability.SHUTDOWN)) + continue; + break; + default: + break; + } + break; + } return new FutureCallback(true); } @@ -786,7 +808,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public boolean isAvailable() { - return _availability == Availability.AVAILABLE; + return _availability.get() == Availability.AVAILABLE; } /** @@ -796,12 +818,46 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setAvailable(boolean available) { - synchronized (this) + // Only supported state transitions are: + // UNAVAILABLE --true---> AVAILABLE + // STARTING -----false--> UNAVAILABLE + // AVAILABLE ----false--> UNAVAILABLE + if (available) { - if (available && isRunning()) - _availability = Availability.AVAILABLE; - else if (!available || !isRunning()) - _availability = Availability.UNAVAILABLE; + while (true) + { + Availability availability = _availability.get(); + switch (availability) + { + case AVAILABLE: + break; + case UNAVAILABLE: + if (!_availability.compareAndSet(availability, Availability.AVAILABLE)) + continue; + break; + default: + throw new IllegalStateException(availability.toString()); + } + break; + } + } + else + { + while (true) + { + Availability availability = _availability.get(); + switch (availability) + { + case STARTING: + case AVAILABLE: + if (!_availability.compareAndSet(availability, Availability.UNAVAILABLE)) + continue; + break; + default: + break; + } + break; + } } } @@ -821,7 +877,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @Override protected void doStart() throws Exception { - _availability = Availability.STARTING; + _availability.set(Availability.STARTING); if (_contextPath == null) throw new IllegalStateException("Null contextPath"); @@ -856,13 +912,12 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu contextInitialized(); - _availability = Availability.AVAILABLE; + _availability.compareAndSet(Availability.STARTING, Availability.AVAILABLE); LOG.info("Started {}", this); } finally { - if (_availability == Availability.STARTING) - _availability = Availability.UNAVAILABLE; + _availability.compareAndSet(Availability.STARTING, Availability.UNAVAILABLE); exitScope(null); __context.set(oldContext); // reset the classloader @@ -1038,7 +1093,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } } - _availability = Availability.UNAVAILABLE; + _availability.set(Availability.STOPPED); ClassLoader oldClassloader = null; ClassLoader oldWebapploader = null; @@ -1192,8 +1247,10 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu return false; } - switch (_availability) + switch (_availability.get()) { + case STOPPED: + return false; case SHUTDOWN: case UNAVAILABLE: baseRequest.setHandled(true); @@ -1584,6 +1641,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setClassLoader(ClassLoader classLoader) { + if (isStarted()) + throw new IllegalStateException(getState()); _classLoader = classLoader; } @@ -1814,7 +1873,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu b.append('{'); if (getDisplayName() != null) b.append(getDisplayName()).append(','); - b.append(getContextPath()).append(',').append(getBaseResource()).append(',').append(_availability); + b.append(getContextPath()).append(',').append(getBaseResource()).append(',').append(_availability.get()); if (vhosts != null && vhosts.length > 0) b.append(',').append(vhosts[0]); @@ -1823,7 +1882,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu return b.toString(); } - public synchronized Class loadClass(String className) throws ClassNotFoundException + public Class loadClass(String className) throws ClassNotFoundException { if (className == null) return null; @@ -2334,7 +2393,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu * @see javax.servlet.ServletContext#getAttribute(java.lang.String) */ @Override - public synchronized Object getAttribute(String name) + public Object getAttribute(String name) { Object o = ContextHandler.this.getAttribute(name); if (o == null) @@ -2346,7 +2405,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu * @see javax.servlet.ServletContext#getAttributeNames() */ @Override - public synchronized Enumeration getAttributeNames() + public Enumeration getAttributeNames() { HashSet set = new HashSet<>(); Enumeration e = super.getAttributeNames(); @@ -2367,7 +2426,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu * @see javax.servlet.ServletContext#setAttribute(java.lang.String, java.lang.Object) */ @Override - public synchronized void setAttribute(String name, Object value) + public void setAttribute(String name, Object value) { Object oldValue = super.getAttribute(name); @@ -2396,7 +2455,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu * @see javax.servlet.ServletContext#removeAttribute(java.lang.String) */ @Override - public synchronized void removeAttribute(String name) + public void removeAttribute(String name) { Object oldValue = super.getAttribute(name); super.removeAttribute(name); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java index 7f702540071..63ef7265016 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java @@ -417,6 +417,18 @@ public class ContextHandlerTest assertThat(connector.getResponse("GET /foo/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo'")); assertThat(connector.getResponse("GET /foo/bar/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo/bar'")); + // If we make foobar unavailable, then requests will be handled by 503 + foobar.setAvailable(false); + assertThat(connector.getResponse("GET / HTTP/1.0\n\n"), Matchers.containsString("ctx=''")); + assertThat(connector.getResponse("GET /foo/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo'")); + assertThat(connector.getResponse("GET /foo/bar/xxx HTTP/1.0\n\n"), Matchers.containsString(" 503 ")); + + // If we make foobar available, then requests will be handled normally + foobar.setAvailable(true); + assertThat(connector.getResponse("GET / HTTP/1.0\n\n"), Matchers.containsString("ctx=''")); + assertThat(connector.getResponse("GET /foo/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo'")); + assertThat(connector.getResponse("GET /foo/bar/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo/bar'")); + // If we stop foobar, then requests will be handled by foo foobar.stop(); assertThat(connector.getResponse("GET / HTTP/1.0\n\n"), Matchers.containsString("ctx=''")); diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java index 830f4ff23f4..1c4454ad36e 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java @@ -114,6 +114,13 @@ public class URIUtilCanonicalPathTest // paths with encoded segments should remain encoded // canonicalPath() is not responsible for decoding characters {"%2e%2e/", "%2e%2e/"}, + {"/%2e%2e/", "/%2e%2e/"}, + + // paths with parameters are not elided + // canonicalPath() is not responsible for decoding characters + {"/foo/.;/bar", "/foo/.;/bar"}, + {"/foo/..;/bar", "/foo/..;/bar"}, + {"/foo/..;/..;/bar", "/foo/..;/..;/bar"}, }; ArrayList ret = new ArrayList<>(); diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index 13b8173f9dd..14df9557bd6 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -1434,7 +1434,6 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL setClassLoader(null); } - setAvailable(true); _unavailableException = null; } }