diff --git a/runtime/local_jetty121/src/main/java/com/google/appengine/tools/development/jetty/AppEngineWebAppContext.java b/runtime/local_jetty121/src/main/java/com/google/appengine/tools/development/jetty/AppEngineWebAppContext.java index b62e5e587..2e5ee9c3c 100644 --- a/runtime/local_jetty121/src/main/java/com/google/appengine/tools/development/jetty/AppEngineWebAppContext.java +++ b/runtime/local_jetty121/src/main/java/com/google/appengine/tools/development/jetty/AppEngineWebAppContext.java @@ -27,6 +27,7 @@ import org.eclipse.jetty.ee8.security.RoleInfo; import org.eclipse.jetty.ee8.security.SecurityHandler; import org.eclipse.jetty.ee8.security.UserDataConstraint; +import org.eclipse.jetty.ee8.servlet.ServletHandler; import org.eclipse.jetty.ee8.webapp.WebAppContext; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; @@ -70,6 +71,8 @@ public AppEngineWebAppContext(File appDir, String serverInfo) { this.serverInfo = serverInfo; + setThrowUnavailableOnStartupException(true); + // Configure the Jetty SecurityHandler to understand our method of // authentication (via the UserService). AppEngineAuthentication.configureSecurityHandler( @@ -78,6 +81,57 @@ public AppEngineWebAppContext(File appDir, String serverInfo) { setMaxFormContentSize(MAX_RESPONSE_SIZE); } + /** + * Configures the {@link ServletHandler} to disallow starting with unavailable servlets or + * filters. + * + *

Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter + * initialization failure throws an exception up the startup lifecycle chain rather than silently + * marking the handler component as unavailable. + */ + @Override + protected ServletHandler newServletHandler() { + ServletHandler handler = new ServletHandler(); + handler.setStartWithUnavailable(false); + return handler; + } + + /** + * Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet + * class defined in {@code web.xml}) are reported as fatal startup exceptions. + * + *

By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException} + * during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as + * unavailable without failing context startup. We inspect the context and all registered + * servlets; if any unavailable exception was caught during startup, we rethrow it immediately so + * application deployment terminates rather than serving HTTP 503 errors at runtime. + * + * @throws Exception if the context or any of its servlets fail to initialize. + * @see Issue #103 + */ + @Override + protected void doStart() throws Exception { + super.doStart(); + Throwable t = getUnavailableException(); + if (t != null) { + if (t instanceof Exception) { + throw (Exception) t; + } + if (t instanceof Error) { + throw (Error) t; + } + throw new IllegalStateException("Context initialization failed", t); + } + ServletHandler servletHandler = getServletHandler(); + if (servletHandler != null && servletHandler.getServlets() != null) { + for (var holder : servletHandler.getServlets()) { + if (holder.getUnavailableException() != null) { + throw holder.getUnavailableException(); + } + } + } + } + @Override public APIContext getServletContext() { // TODO: Override the default HttpServletContext implementation (for logging)?. diff --git a/runtime/local_jetty121_ee11/src/main/java/com/google/appengine/tools/development/jetty/ee11/AppEngineWebAppContext.java b/runtime/local_jetty121_ee11/src/main/java/com/google/appengine/tools/development/jetty/ee11/AppEngineWebAppContext.java index 8e84f0709..00190af85 100644 --- a/runtime/local_jetty121_ee11/src/main/java/com/google/appengine/tools/development/jetty/ee11/AppEngineWebAppContext.java +++ b/runtime/local_jetty121_ee11/src/main/java/com/google/appengine/tools/development/jetty/ee11/AppEngineWebAppContext.java @@ -19,6 +19,7 @@ import com.google.apphosting.api.ApiProxy; import com.google.apphosting.runtime.jetty.EE11AppEngineAuthentication; import java.io.File; +import org.eclipse.jetty.ee11.servlet.ServletHandler; import org.eclipse.jetty.ee11.servlet.security.ConstraintSecurityHandler; import org.eclipse.jetty.ee11.webapp.WebAppContext; import org.eclipse.jetty.security.Constraint; @@ -66,6 +67,8 @@ public AppEngineWebAppContext(File appDir, String serverInfo) { this.serverInfo = serverInfo; + setThrowUnavailableOnStartupException(true); + // Configure the Jetty SecurityHandler to understand our method of // authentication (via the UserService). setSecurityHandler(EE11AppEngineAuthentication.newSecurityHandler()); @@ -73,6 +76,57 @@ public AppEngineWebAppContext(File appDir, String serverInfo) { setMaxFormContentSize(MAX_RESPONSE_SIZE); } + /** + * Configures the {@link ServletHandler} to disallow starting with unavailable servlets or + * filters. + * + *

Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter + * initialization failure throws an exception up the startup lifecycle chain rather than silently + * marking the handler component as unavailable. + */ + @Override + protected ServletHandler newServletHandler() { + ServletHandler handler = new ServletHandler(); + handler.setStartWithUnavailable(false); + return handler; + } + + /** + * Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet + * class defined in {@code web.xml}) are reported as fatal startup exceptions. + * + *

By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException} + * during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as + * unavailable without failing context startup. We inspect the context and all registered + * servlets; if any unavailable exception was caught during startup, we rethrow it immediately so + * application deployment terminates rather than serving HTTP 503 errors at runtime. + * + * @throws Exception if the context or any of its servlets fail to initialize. + * @see Issue #103 + */ + @Override + protected void doStart() throws Exception { + super.doStart(); + Throwable t = getUnavailableException(); + if (t != null) { + if (t instanceof Exception) { + throw (Exception) t; + } + if (t instanceof Error) { + throw (Error) t; + } + throw new IllegalStateException("Context initialization failed", t); + } + ServletHandler servletHandler = getServletHandler(); + if (servletHandler != null && servletHandler.getServlets() != null) { + for (var holder : servletHandler.getServlets()) { + if (holder.getUnavailableException() != null) { + throw holder.getUnavailableException(); + } + } + } + } + @Override public ServletScopedContext getContext() { // TODO: Override the default HttpServletContext implementation (for logging)?. diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java index 678d65fb2..554101974 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java @@ -203,9 +203,40 @@ public boolean removeEventListener(EventListener listener) { return false; } + /** + * Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet + * class defined in {@code web.xml}) are reported as fatal startup exceptions. + * + *

By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException} + * during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as + * unavailable without failing context startup. We inspect the context and all registered + * servlets; if any unavailable exception was caught during startup, we rethrow it immediately so + * application deployment terminates rather than serving HTTP 503 errors at runtime. + * + * @throws Exception if the context or any of its servlets fail to initialize. + * @see Issue #103 + */ @Override public void doStart() throws Exception { super.doStart(); + Throwable t = getUnavailableException(); + if (t != null) { + if (t instanceof Exception) { + throw (Exception) t; + } + if (t instanceof Error) { + throw (Error) t; + } + throw new IllegalStateException("Context initialization failed", t); + } + ServletHandler servletHandler = getServletHandler(); + if (servletHandler != null && servletHandler.getServlets() != null) { + for (var holder : servletHandler.getServlets()) { + if (holder.getUnavailableException() != null) { + throw holder.getUnavailableException(); + } + } + } addEventListener(new TransactionCleanupListener(getClassLoader())); } @@ -275,10 +306,19 @@ public boolean handle(Request request, Response response, Callback callback) thr } } + /** + * Configures the {@link ServletHandler} to disallow starting with unavailable servlets or + * filters. + * + *

Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter + * initialization failure throws an exception up the startup lifecycle chain rather than silently + * marking the handler component as unavailable. + */ @Override protected ServletHandler newServletHandler() { ServletHandler handler = new ServletHandler(); handler.setAllowDuplicateMappings(true); + handler.setStartWithUnavailable(false); if (AppEngineConstants.isLegacyMode()) { handler.setDecodeAmbiguousURIs(true); } diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java index f48353b01..0d94a75ae 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java @@ -235,9 +235,40 @@ public boolean removeEventListener(EventListener listener) { return false; } + /** + * Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet + * class defined in {@code web.xml}) are reported as fatal startup exceptions. + * + *

By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException} + * during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as + * unavailable without failing context startup. We inspect the context and all registered + * servlets; if any unavailable exception was caught during startup, we rethrow it immediately so + * application deployment terminates rather than serving HTTP 503 errors at runtime. + * + * @throws Exception if the context or any of its servlets fail to initialize. + * @see Issue #103 + */ @Override public void doStart() throws Exception { super.doStart(); + Throwable t = getUnavailableException(); + if (t != null) { + if (t instanceof Exception) { + throw (Exception) t; + } + if (t instanceof Error) { + throw (Error) t; + } + throw new IllegalStateException("Context initialization failed", t); + } + ServletHandler servletHandler = getServletHandler(); + if (servletHandler != null && servletHandler.getServlets() != null) { + for (var holder : servletHandler.getServlets()) { + if (holder.getUnavailableException() != null) { + throw holder.getUnavailableException(); + } + } + } addEventListener(new TransactionCleanupListener(getClassLoader())); } @@ -312,10 +343,19 @@ public void doHandle( } } + /** + * Configures the {@link ServletHandler} to disallow starting with unavailable servlets or + * filters. + * + *

Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter + * initialization failure throws an exception up the startup lifecycle chain rather than silently + * marking the handler component as unavailable. + */ @Override protected ServletHandler newServletHandler() { ServletHandler handler = new ServletHandler(); handler.setAllowDuplicateMappings(true); + handler.setStartWithUnavailable(false); return handler; } diff --git a/runtime/runtime_impl_jetty12/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java b/runtime/runtime_impl_jetty12/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java index e7ed9bfb1..efb7aa028 100644 --- a/runtime/runtime_impl_jetty12/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java +++ b/runtime/runtime_impl_jetty12/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.appengine.tools.development.resource.ResourceExtractor; @@ -27,6 +28,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.jar.JarEntry; @@ -133,4 +135,55 @@ public void doesntExtractWar() throws Exception { assertEquals(files.length, 0); } } + + @Test + public void missingServletClassThrowsOnStartupEe8() throws Exception { + File appDir = temporaryFolder.newFolder("missingservletapp-ee8"); + File webInf = new File(appDir, "WEB-INF"); + webInf.mkdirs(); + File webXml = new File(webInf, "web.xml"); + Files.writeString( + webXml.toPath(), + "\n" + + " \n" + + " MissingServlet\n" + + " com.example.nonexistent.MissingServlet\n" + + " \n" + + " \n" + + " MissingServlet\n" + + " /missing\n" + + " \n" + + "\n"); + + AppEngineWebAppContext context = new AppEngineWebAppContext(appDir, "test server"); + context.setTempDirectory(new File(appDir, "tmp")); + context.setServer(new org.eclipse.jetty.server.Server()); + assertThrows(Exception.class, context::doStart); + } + + @Test + public void missingServletClassThrowsOnStartupEe10() throws Exception { + File appDir = temporaryFolder.newFolder("missingservletapp-ee10"); + File webInf = new File(appDir, "WEB-INF"); + webInf.mkdirs(); + File webXml = new File(webInf, "web.xml"); + Files.writeString( + webXml.toPath(), + "\n" + + " \n" + + " MissingServlet\n" + + " com.example.nonexistent.MissingServlet\n" + + " \n" + + " \n" + + " MissingServlet\n" + + " /missing\n" + + " \n" + + "\n"); + + com.google.apphosting.runtime.jetty.ee10.AppEngineWebAppContext context = + new com.google.apphosting.runtime.jetty.ee10.AppEngineWebAppContext(appDir, "test server", true); + context.setTempDirectory(new File(appDir, "tmp")); + context.setServer(new org.eclipse.jetty.server.Server()); + assertThrows(Exception.class, context::doStart); + } } diff --git a/runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee11/AppEngineWebAppContext.java b/runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee11/AppEngineWebAppContext.java index 98b1721cf..2647a7dc3 100644 --- a/runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee11/AppEngineWebAppContext.java +++ b/runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee11/AppEngineWebAppContext.java @@ -203,9 +203,40 @@ public boolean removeEventListener(EventListener listener) { return false; } + /** + * Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet + * class defined in {@code web.xml}) are reported as fatal startup exceptions. + * + *

By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException} + * during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as + * unavailable without failing context startup. We inspect the context and all registered + * servlets; if any unavailable exception was caught during startup, we rethrow it immediately so + * application deployment terminates rather than serving HTTP 503 errors at runtime. + * + * @throws Exception if the context or any of its servlets fail to initialize. + * @see Issue #103 + */ @Override public void doStart() throws Exception { super.doStart(); + Throwable t = getUnavailableException(); + if (t != null) { + if (t instanceof Exception) { + throw (Exception) t; + } + if (t instanceof Error) { + throw (Error) t; + } + throw new IllegalStateException("Context initialization failed", t); + } + ServletHandler servletHandler = getServletHandler(); + if (servletHandler != null && servletHandler.getServlets() != null) { + for (var holder : servletHandler.getServlets()) { + if (holder.getUnavailableException() != null) { + throw holder.getUnavailableException(); + } + } + } addEventListener(new TransactionCleanupListener(getClassLoader())); } @@ -275,10 +306,19 @@ public boolean handle(Request request, Response response, Callback callback) thr } } + /** + * Configures the {@link ServletHandler} to disallow starting with unavailable servlets or + * filters. + * + *

Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter + * initialization failure throws an exception up the startup lifecycle chain rather than silently + * marking the handler component as unavailable. + */ @Override protected ServletHandler newServletHandler() { ServletHandler handler = new ServletHandler(); handler.setAllowDuplicateMappings(true); + handler.setStartWithUnavailable(false); if (AppEngineConstants.isLegacyMode()) { handler.setDecodeAmbiguousURIs(true); } diff --git a/runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java b/runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java index c582787aa..ee2a99c47 100644 --- a/runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java +++ b/runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java @@ -238,9 +238,40 @@ public boolean removeEventListener(EventListener listener) { return false; } + /** + * Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet + * class defined in {@code web.xml}) are reported as fatal startup exceptions. + * + *

By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException} + * during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as + * unavailable without failing context startup. We inspect the context and all registered + * servlets; if any unavailable exception was caught during startup, we rethrow it immediately so + * application deployment terminates rather than serving HTTP 503 errors at runtime. + * + * @throws Exception if the context or any of its servlets fail to initialize. + * @see Issue #103 + */ @Override public void doStart() throws Exception { super.doStart(); + Throwable t = getUnavailableException(); + if (t != null) { + if (t instanceof Exception) { + throw (Exception) t; + } + if (t instanceof Error) { + throw (Error) t; + } + throw new IllegalStateException("Context initialization failed", t); + } + ServletHandler servletHandler = getServletHandler(); + if (servletHandler != null && servletHandler.getServlets() != null) { + for (var holder : servletHandler.getServlets()) { + if (holder.getUnavailableException() != null) { + throw holder.getUnavailableException(); + } + } + } var unused = addEventListener(new TransactionCleanupListener(getClassLoader())); } @@ -315,10 +346,19 @@ public void doHandle( } } + /** + * Configures the {@link ServletHandler} to disallow starting with unavailable servlets or + * filters. + * + *

Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter + * initialization failure throws an exception up the startup lifecycle chain rather than silently + * marking the handler component as unavailable. + */ @Override protected ServletHandler newServletHandler() { ServletHandler handler = new ServletHandler(); handler.setAllowDuplicateMappings(true); + handler.setStartWithUnavailable(false); return handler; } diff --git a/runtime/runtime_impl_jetty121/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java b/runtime/runtime_impl_jetty121/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java index eded28e6b..3de17922c 100644 --- a/runtime/runtime_impl_jetty121/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java +++ b/runtime/runtime_impl_jetty121/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import com.google.appengine.tools.development.resource.ResourceExtractor; @@ -26,6 +27,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.jar.JarEntry;