Skip to content

Commit b03e329

Browse files
ludochgae-java-bot
ludoch
authored and
gae-java-bot
committed
Copybara import of the project:
-- 40271f8 by Ludovic Champenois <ludo@google.com>: fix(jetty12): throw fatal exception when webapp servlets fail to initialize (#103) -- PiperOrigin-RevId: 955866664 Change-Id: Ie790c85a0e671245bc4ed45fe5a1fc5cef2eea0a
1 parent 2288471 commit b03e329

7 files changed

Lines changed: 323 additions & 3 deletions

File tree

‎runtime/local_jetty121/src/main/java/com/google/appengine/tools/development/jetty/AppEngineWebAppContext.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.eclipse.jetty.ee8.security.RoleInfo;
2828
import org.eclipse.jetty.ee8.security.SecurityHandler;
2929
import org.eclipse.jetty.ee8.security.UserDataConstraint;
30+
import org.eclipse.jetty.ee8.servlet.ServletHandler;
3031
import org.eclipse.jetty.ee8.webapp.WebAppContext;
3132
import org.eclipse.jetty.util.resource.Resource;
3233
import org.eclipse.jetty.util.resource.ResourceFactory;
@@ -70,6 +71,8 @@ public AppEngineWebAppContext(File appDir, String serverInfo) {
7071

7172
this.serverInfo = serverInfo;
7273

74+
setThrowUnavailableOnStartupException(true);
75+
7376
// Configure the Jetty SecurityHandler to understand our method of
7477
// authentication (via the UserService).
7578
AppEngineAuthentication.configureSecurityHandler(
@@ -78,6 +81,57 @@ public AppEngineWebAppContext(File appDir, String serverInfo) {
7881
setMaxFormContentSize(MAX_RESPONSE_SIZE);
7982
}
8083

84+
/**
85+
* Configures the {@link ServletHandler} to disallow starting with unavailable servlets or
86+
* filters.
87+
*
88+
* <p>Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter
89+
* initialization failure throws an exception up the startup lifecycle chain rather than silently
90+
* marking the handler component as unavailable.
91+
*/
92+
@Override
93+
protected ServletHandler newServletHandler() {
94+
ServletHandler handler = new ServletHandler();
95+
handler.setStartWithUnavailable(false);
96+
return handler;
97+
}
98+
99+
/**
100+
* Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet
101+
* class defined in {@code web.xml}) are reported as fatal startup exceptions.
102+
*
103+
* <p>By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException}
104+
* during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as
105+
* unavailable without failing context startup. We inspect the context and all registered
106+
* servlets; if any unavailable exception was caught during startup, we rethrow it immediately so
107+
* application deployment terminates rather than serving HTTP 503 errors at runtime.
108+
*
109+
* @throws Exception if the context or any of its servlets fail to initialize.
110+
* @see <a href="https://github.com/GoogleCloudPlatform/appengine-java-standard/issues/103">Issue #103</a>
111+
*/
112+
@Override
113+
protected void doStart() throws Exception {
114+
super.doStart();
115+
Throwable t = getUnavailableException();
116+
if (t != null) {
117+
if (t instanceof Exception) {
118+
throw (Exception) t;
119+
}
120+
if (t instanceof Error) {
121+
throw (Error) t;
122+
}
123+
throw new IllegalStateException("Context initialization failed", t);
124+
}
125+
ServletHandler servletHandler = getServletHandler();
126+
if (servletHandler != null && servletHandler.getServlets() != null) {
127+
for (var holder : servletHandler.getServlets()) {
128+
if (holder.getUnavailableException() != null) {
129+
throw holder.getUnavailableException();
130+
}
131+
}
132+
}
133+
}
134+
81135
@Override
82136
public APIContext getServletContext() {
83137
// TODO: Override the default HttpServletContext implementation (for logging)?.

‎runtime/local_jetty121_ee11/src/main/java/com/google/appengine/tools/development/jetty/ee11/AppEngineWebAppContext.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.apphosting.api.ApiProxy;
2020
import com.google.apphosting.runtime.jetty.EE11AppEngineAuthentication;
2121
import java.io.File;
22+
import org.eclipse.jetty.ee11.servlet.ServletHandler;
2223
import org.eclipse.jetty.ee11.servlet.security.ConstraintSecurityHandler;
2324
import org.eclipse.jetty.ee11.webapp.WebAppContext;
2425
import org.eclipse.jetty.security.Constraint;
@@ -66,13 +67,66 @@ public AppEngineWebAppContext(File appDir, String serverInfo) {
6667

6768
this.serverInfo = serverInfo;
6869

70+
setThrowUnavailableOnStartupException(true);
71+
6972
// Configure the Jetty SecurityHandler to understand our method of
7073
// authentication (via the UserService).
7174
setSecurityHandler(EE11AppEngineAuthentication.newSecurityHandler());
7275

7376
setMaxFormContentSize(MAX_RESPONSE_SIZE);
7477
}
7578

79+
/**
80+
* Configures the {@link ServletHandler} to disallow starting with unavailable servlets or
81+
* filters.
82+
*
83+
* <p>Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter
84+
* initialization failure throws an exception up the startup lifecycle chain rather than silently
85+
* marking the handler component as unavailable.
86+
*/
87+
@Override
88+
protected ServletHandler newServletHandler() {
89+
ServletHandler handler = new ServletHandler();
90+
handler.setStartWithUnavailable(false);
91+
return handler;
92+
}
93+
94+
/**
95+
* Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet
96+
* class defined in {@code web.xml}) are reported as fatal startup exceptions.
97+
*
98+
* <p>By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException}
99+
* during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as
100+
* unavailable without failing context startup. We inspect the context and all registered
101+
* servlets; if any unavailable exception was caught during startup, we rethrow it immediately so
102+
* application deployment terminates rather than serving HTTP 503 errors at runtime.
103+
*
104+
* @throws Exception if the context or any of its servlets fail to initialize.
105+
* @see <a href="https://github.com/GoogleCloudPlatform/appengine-java-standard/issues/103">Issue #103</a>
106+
*/
107+
@Override
108+
protected void doStart() throws Exception {
109+
super.doStart();
110+
Throwable t = getUnavailableException();
111+
if (t != null) {
112+
if (t instanceof Exception) {
113+
throw (Exception) t;
114+
}
115+
if (t instanceof Error) {
116+
throw (Error) t;
117+
}
118+
throw new IllegalStateException("Context initialization failed", t);
119+
}
120+
ServletHandler servletHandler = getServletHandler();
121+
if (servletHandler != null && servletHandler.getServlets() != null) {
122+
for (var holder : servletHandler.getServlets()) {
123+
if (holder.getUnavailableException() != null) {
124+
throw holder.getUnavailableException();
125+
}
126+
}
127+
}
128+
}
129+
76130
@Override
77131
public ServletScopedContext getContext() {
78132
// TODO: Override the default HttpServletContext implementation (for logging)?.

‎runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,9 +203,40 @@ public boolean removeEventListener(EventListener listener) {
203203
return false;
204204
}
205205

206+
/**
207+
* Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet
208+
* class defined in {@code web.xml}) are reported as fatal startup exceptions.
209+
*
210+
* <p>By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException}
211+
* during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as
212+
* unavailable without failing context startup. We inspect the context and all registered
213+
* servlets; if any unavailable exception was caught during startup, we rethrow it immediately so
214+
* application deployment terminates rather than serving HTTP 503 errors at runtime.
215+
*
216+
* @throws Exception if the context or any of its servlets fail to initialize.
217+
* @see <a href="https://github.com/GoogleCloudPlatform/appengine-java-standard/issues/103">Issue #103</a>
218+
*/
206219
@Override
207220
public void doStart() throws Exception {
208221
super.doStart();
222+
Throwable t = getUnavailableException();
223+
if (t != null) {
224+
if (t instanceof Exception) {
225+
throw (Exception) t;
226+
}
227+
if (t instanceof Error) {
228+
throw (Error) t;
229+
}
230+
throw new IllegalStateException("Context initialization failed", t);
231+
}
232+
ServletHandler servletHandler = getServletHandler();
233+
if (servletHandler != null && servletHandler.getServlets() != null) {
234+
for (var holder : servletHandler.getServlets()) {
235+
if (holder.getUnavailableException() != null) {
236+
throw holder.getUnavailableException();
237+
}
238+
}
239+
}
209240
addEventListener(new TransactionCleanupListener(getClassLoader()));
210241
}
211242

@@ -275,10 +306,19 @@ public boolean handle(Request request, Response response, Callback callback) thr
275306
}
276307
}
277308

309+
/**
310+
* Configures the {@link ServletHandler} to disallow starting with unavailable servlets or
311+
* filters.
312+
*
313+
* <p>Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter
314+
* initialization failure throws an exception up the startup lifecycle chain rather than silently
315+
* marking the handler component as unavailable.
316+
*/
278317
@Override
279318
protected ServletHandler newServletHandler() {
280319
ServletHandler handler = new ServletHandler();
281320
handler.setAllowDuplicateMappings(true);
321+
handler.setStartWithUnavailable(false);
282322
if (AppEngineConstants.isLegacyMode()) {
283323
handler.setDecodeAmbiguousURIs(true);
284324
}

‎runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,40 @@ public boolean removeEventListener(EventListener listener) {
235235
return false;
236236
}
237237

238+
/**
239+
* Overrides {@code doStart} to ensure that any initialization errors (such as a missing servlet
240+
* class defined in {@code web.xml}) are reported as fatal startup exceptions.
241+
*
242+
* <p>By default, Jetty may catch {@link ClassNotFoundException} or {@link UnavailableException}
243+
* during {@link ServletHandler#initialize()} and mark the individual {@code ServletHolder} as
244+
* unavailable without failing context startup. We inspect the context and all registered
245+
* servlets; if any unavailable exception was caught during startup, we rethrow it immediately so
246+
* application deployment terminates rather than serving HTTP 503 errors at runtime.
247+
*
248+
* @throws Exception if the context or any of its servlets fail to initialize.
249+
* @see <a href="https://github.com/GoogleCloudPlatform/appengine-java-standard/issues/103">Issue #103</a>
250+
*/
238251
@Override
239252
public void doStart() throws Exception {
240253
super.doStart();
254+
Throwable t = getUnavailableException();
255+
if (t != null) {
256+
if (t instanceof Exception) {
257+
throw (Exception) t;
258+
}
259+
if (t instanceof Error) {
260+
throw (Error) t;
261+
}
262+
throw new IllegalStateException("Context initialization failed", t);
263+
}
264+
ServletHandler servletHandler = getServletHandler();
265+
if (servletHandler != null && servletHandler.getServlets() != null) {
266+
for (var holder : servletHandler.getServlets()) {
267+
if (holder.getUnavailableException() != null) {
268+
throw holder.getUnavailableException();
269+
}
270+
}
271+
}
241272
addEventListener(new TransactionCleanupListener(getClassLoader()));
242273
}
243274

@@ -312,10 +343,19 @@ public void doHandle(
312343
}
313344
}
314345

346+
/**
347+
* Configures the {@link ServletHandler} to disallow starting with unavailable servlets or
348+
* filters.
349+
*
350+
* <p>Setting {@code setStartWithUnavailable(false)} ensures that any servlet or filter
351+
* initialization failure throws an exception up the startup lifecycle chain rather than silently
352+
* marking the handler component as unavailable.
353+
*/
315354
@Override
316355
protected ServletHandler newServletHandler() {
317356
ServletHandler handler = new ServletHandler();
318357
handler.setAllowDuplicateMappings(true);
358+
handler.setStartWithUnavailable(false);
319359
return handler;
320360
}
321361

‎runtime/runtime_impl_jetty12/src/test/java/com/google/apphosting/runtime/jetty/AppEngineWebAppContextTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020
import static org.junit.jupiter.api.Assertions.assertEquals;
2121
import static org.junit.jupiter.api.Assertions.assertNotNull;
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
2223
import static org.junit.jupiter.api.Assertions.assertTrue;
2324

2425
import com.google.appengine.tools.development.resource.ResourceExtractor;
@@ -27,6 +28,7 @@
2728
import java.io.File;
2829
import java.io.FileInputStream;
2930
import java.io.FileOutputStream;
31+
import java.nio.file.Files;
3032
import java.nio.file.Path;
3133
import java.nio.file.Paths;
3234
import java.util.jar.JarEntry;
@@ -133,4 +135,55 @@ public void doesntExtractWar() throws Exception {
133135
assertEquals(files.length, 0);
134136
}
135137
}
138+
139+
@Test
140+
public void missingServletClassThrowsOnStartupEe8() throws Exception {
141+
File appDir = temporaryFolder.newFolder("missingservletapp-ee8");
142+
File webInf = new File(appDir, "WEB-INF");
143+
webInf.mkdirs();
144+
File webXml = new File(webInf, "web.xml");
145+
Files.writeString(
146+
webXml.toPath(),
147+
"<web-app xmlns=\"http://xmlns.jcp.org/xml/ns/javaee\" version=\"4.0\">\n"
148+
+ " <servlet>\n"
149+
+ " <servlet-name>MissingServlet</servlet-name>\n"
150+
+ " <servlet-class>com.example.nonexistent.MissingServlet</servlet-class>\n"
151+
+ " </servlet>\n"
152+
+ " <servlet-mapping>\n"
153+
+ " <servlet-name>MissingServlet</servlet-name>\n"
154+
+ " <url-pattern>/missing</url-pattern>\n"
155+
+ " </servlet-mapping>\n"
156+
+ "</web-app>\n");
157+
158+
AppEngineWebAppContext context = new AppEngineWebAppContext(appDir, "test server");
159+
context.setTempDirectory(new File(appDir, "tmp"));
160+
context.setServer(new org.eclipse.jetty.server.Server());
161+
assertThrows(Exception.class, context::doStart);
162+
}
163+
164+
@Test
165+
public void missingServletClassThrowsOnStartupEe10() throws Exception {
166+
File appDir = temporaryFolder.newFolder("missingservletapp-ee10");
167+
File webInf = new File(appDir, "WEB-INF");
168+
webInf.mkdirs();
169+
File webXml = new File(webInf, "web.xml");
170+
Files.writeString(
171+
webXml.toPath(),
172+
"<web-app xmlns=\"https://jakarta.ee/xml/ns/jakartaee\" version=\"6.0\">\n"
173+
+ " <servlet>\n"
174+
+ " <servlet-name>MissingServlet</servlet-name>\n"
175+
+ " <servlet-class>com.example.nonexistent.MissingServlet</servlet-class>\n"
176+
+ " </servlet>\n"
177+
+ " <servlet-mapping>\n"
178+
+ " <servlet-name>MissingServlet</servlet-name>\n"
179+
+ " <url-pattern>/missing</url-pattern>\n"
180+
+ " </servlet-mapping>\n"
181+
+ "</web-app>\n");
182+
183+
com.google.apphosting.runtime.jetty.ee10.AppEngineWebAppContext context =
184+
new com.google.apphosting.runtime.jetty.ee10.AppEngineWebAppContext(appDir, "test server", true);
185+
context.setTempDirectory(new File(appDir, "tmp"));
186+
context.setServer(new org.eclipse.jetty.server.Server());
187+
assertThrows(Exception.class, context::doStart);
188+
}
136189
}

0 commit comments

Comments
 (0)