From 6fac4e9ec81863012ad94d68dabc714e0a4576ec Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 13 Nov 2024 12:23:14 +1100 Subject: [PATCH 1/4] fix GzipHandler issues with HttpConnector mode enabled on Jetty 9.4 Signed-off-by: Lachlan Roberts --- .../runtime/jetty9/JettyRequestAPIData.java | 21 ++- .../jetty9/JettyServletEngineAdapter.java | 3 +- .../runtime/jetty9/GzipHandlerTest.java | 147 ++++++++++++++++++ runtime/testapps/pom.xml | 4 + .../jetty9/gzipapp/EE10EchoServlet.java | 45 ++++++ .../jetty9/gzipapp/EE8EchoServlet.java | 45 ++++++ .../gzipapp/ee10/WEB-INF/appengine-web.xml | 24 +++ .../jetty9/gzipapp/ee10/WEB-INF/web.xml | 32 ++++ .../gzipapp/ee8/WEB-INF/appengine-web.xml | 24 +++ .../jetty9/gzipapp/ee8/WEB-INF/web.xml | 32 ++++ .../gzipapp/jetty94/WEB-INF/appengine-web.xml | 25 +++ .../jetty9/gzipapp/jetty94/WEB-INF/web.xml | 32 ++++ 12 files changed, 425 insertions(+), 9 deletions(-) create mode 100644 runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/GzipHandlerTest.java create mode 100644 runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/gzipapp/EE10EchoServlet.java create mode 100644 runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/gzipapp/EE8EchoServlet.java create mode 100644 runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee10/WEB-INF/appengine-web.xml create mode 100644 runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee10/WEB-INF/web.xml create mode 100644 runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee8/WEB-INF/appengine-web.xml create mode 100644 runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee8/WEB-INF/web.xml create mode 100644 runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/jetty94/WEB-INF/appengine-web.xml create mode 100644 runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/jetty94/WEB-INF/web.xml diff --git a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java index f14811a0..cb217fd1 100644 --- a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java +++ b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java @@ -68,6 +68,8 @@ import java.util.stream.Stream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; + +import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpURI; @@ -126,15 +128,20 @@ public JettyRequestAPIData( this.securityTicket = DEFAULT_SECRET_KEY; HttpFields fields = new HttpFields(); - List headerNames = Collections.list(request.getHeaderNames()); - for (String headerName : headerNames) { - String name = headerName.toLowerCase(Locale.ROOT); - String value = request.getHeader(headerName); + for (HttpField field : request.getHttpFields()) { + // If it has a HttpHeader it is one of the standard headers so won't match any appengine specific header. + if (field.getHeader() != null) { + fields.add(field); + continue; + } + + String lowerCaseName = field.getName().toLowerCase(Locale.ROOT); + String value = field.getValue(); if (Strings.isNullOrEmpty(value)) { continue; } - switch (name) { + switch (lowerCaseName) { case X_APPENGINE_TRUSTED_IP_REQUEST: // If there is a value, then the application is trusted // If the value is IS_TRUSTED, then the user is trusted @@ -236,9 +243,9 @@ public JettyRequestAPIData( break; } - if (passThroughPrivateHeaders || !PRIVATE_APPENGINE_HEADERS.contains(name)) { + if (passThroughPrivateHeaders || !PRIVATE_APPENGINE_HEADERS.contains(lowerCaseName)) { // Only non AppEngine specific headers are passed to the application. - fields.add(name, value); + fields.add(field); } } diff --git a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java index d39d9bbe..86644f4e 100644 --- a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java +++ b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java @@ -43,7 +43,6 @@ import java.util.Optional; import javax.servlet.ServletException; import org.eclipse.jetty.server.Connector; -import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.SizeLimitHandler; @@ -141,10 +140,10 @@ public void start(String serverInfo, ServletEngineAdapter.Config runtimeOptions) if (Boolean.getBoolean(HTTP_CONNECTOR_MODE)) { logger.atInfo().log("Using HTTP_CONNECTOR_MODE to bypass RPC"); appVersionKey = AppVersionKey.fromAppInfo(appinfo); - JettyHttpProxy.insertHandlers(server); AppVersion appVersion = appVersionHandlerMap.getAppVersion(appVersionKey); server.insertHandler( new JettyHttpHandler(runtimeOptions, appVersion, appVersionKey, appInfoFactory)); + JettyHttpProxy.insertHandlers(server); ServerConnector connector = JettyHttpProxy.newConnector(server, runtimeOptions); server.addConnector(connector); } else { diff --git a/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/GzipHandlerTest.java b/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/GzipHandlerTest.java new file mode 100644 index 00000000..a9d1dcf1 --- /dev/null +++ b/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/GzipHandlerTest.java @@ -0,0 +1,147 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.apphosting.runtime.jetty9; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.Arrays; +import java.util.Collection; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.zip.GZIPOutputStream; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.ContentProvider; +import org.eclipse.jetty.client.api.Result; +import org.eclipse.jetty.client.util.InputStreamContentProvider; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.util.Utf8StringBuilder; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class GzipHandlerTest extends JavaRuntimeViaHttpBase { + + @Parameterized.Parameters + public static Collection data() { + return Arrays.asList( + new Object[][] { + {"jetty94", false}, + {"jetty94", true}, + {"ee8", false}, + {"ee8", true}, + {"ee10", false}, + {"ee10", true}, + }); + } + + private static final int MAX_SIZE = 32 * 1024 * 1024; + + @Rule public TemporaryFolder temp = new TemporaryFolder(); + private final HttpClient httpClient = new HttpClient(); + private final boolean httpMode; + private final String environment; + private RuntimeContext runtime; + + public GzipHandlerTest(String environment, boolean httpMode) { + this.environment = environment; + this.httpMode = httpMode; + System.setProperty("appengine.use.HttpConnector", Boolean.toString(httpMode)); + } + + @Before + public void before() throws Exception { + String app = "com/google/apphosting/runtime/jetty9/gzipapp/" + environment; + copyAppToDir(app, temp.getRoot().toPath()); + httpClient.start(); + runtime = runtimeContext(); + System.err.println("==== Using Environment: " + environment + " " + httpMode + " ===="); + } + + @After + public void after() throws Exception { + httpClient.stop(); + runtime.close(); + } + + @Test + public void testRequestGzipContent() throws Exception { + int contentLength = 1024; + + CompletableFuture completionListener = new CompletableFuture<>(); + byte[] data = new byte[contentLength]; + Arrays.fill(data, (byte) 'X'); + Utf8StringBuilder received = new Utf8StringBuilder(); + ContentProvider content = new InputStreamContentProvider(gzip(data)); + + String url = runtime.jettyUrl("/"); + httpClient + .newRequest(url) + .content(content) + .onResponseContentAsync( + (response, content1, callback) -> { + received.append(content1); + callback.succeeded(); + }) + .header(HttpHeader.CONTENT_ENCODING, "gzip") + .send(completionListener::complete); + + // The request was successfully decoded by the GzipHandler. + Result response = completionListener.get(5, TimeUnit.SECONDS); + assertThat(response.getResponse().getStatus(), equalTo(HttpStatus.OK_200)); + String contentReceived = received.toString(); + assertThat(contentReceived, containsString("\nX-Content-Encoding: gzip\n")); + assertThat(contentReceived, not(containsString("\nContent-Encoding: gzip\n"))); + assertThat(contentReceived, containsString("\nAccept-Encoding: gzip\n")); + + // Server correctly echoed content of request. + String expectedData = new String(data); + String actualData = contentReceived.substring(contentReceived.length() - contentLength); + assertThat(actualData, equalTo(expectedData)); + + // Response was gzip encoded. + HttpFields responseHeaders = response.getResponse().getHeaders(); + assertThat(responseHeaders.get(HttpHeader.CONTENT_ENCODING), equalTo("gzip")); + } + + private RuntimeContext runtimeContext() throws Exception { + RuntimeContext.Config config = + RuntimeContext.Config.builder().setApplicationPath(temp.getRoot().toString()).build(); + return RuntimeContext.create(config); + } + + private static InputStream gzip(byte[] data) throws IOException { + ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); + try (GZIPOutputStream gzipOutputStream = new GZIPOutputStream(byteArrayOutputStream)) { + gzipOutputStream.write(data); + } + return new ByteArrayInputStream(byteArrayOutputStream.toByteArray()); + } +} diff --git a/runtime/testapps/pom.xml b/runtime/testapps/pom.xml index 30e66d6c..98ebe5ef 100644 --- a/runtime/testapps/pom.xml +++ b/runtime/testapps/pom.xml @@ -36,6 +36,10 @@ com.google.appengine appengine-api-1.0-sdk + + org.eclipse.jetty + jetty-util + com.google.guava guava diff --git a/runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/gzipapp/EE10EchoServlet.java b/runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/gzipapp/EE10EchoServlet.java new file mode 100644 index 00000000..9e838501 --- /dev/null +++ b/runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/gzipapp/EE10EchoServlet.java @@ -0,0 +1,45 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.apphosting.runtime.jetty9.gzipapp; + +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.PrintWriter; +import java.util.Enumeration; +import org.eclipse.jetty.util.IO; + +/** Servlet that prints all the system properties. */ +public class EE10EchoServlet extends HttpServlet { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException { + resp.setContentType("text/plain"); + + PrintWriter writer = resp.getWriter(); + writer.println(); + Enumeration headerNames = req.getHeaderNames(); + while (headerNames.hasMoreElements()) { + String headerName = headerNames.nextElement(); + writer.println(headerName + ": " + req.getHeader(headerName)); + } + writer.println(); + + String string = IO.toString(req.getInputStream()); + writer.print(string); + } +} diff --git a/runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/gzipapp/EE8EchoServlet.java b/runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/gzipapp/EE8EchoServlet.java new file mode 100644 index 00000000..e9a68ac8 --- /dev/null +++ b/runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/gzipapp/EE8EchoServlet.java @@ -0,0 +1,45 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.apphosting.runtime.jetty9.gzipapp; + +import java.io.IOException; +import java.io.PrintWriter; +import java.util.Enumeration; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.util.IO; + +/** Servlet that prints all the system properties. */ +public class EE8EchoServlet extends HttpServlet { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException { + resp.setContentType("text/plain"); + + PrintWriter writer = resp.getWriter(); + writer.println(); + Enumeration headerNames = req.getHeaderNames(); + while (headerNames.hasMoreElements()) { + String headerName = headerNames.nextElement(); + writer.println(headerName + ": " + req.getHeader(headerName)); + } + writer.println(); + + String string = IO.toString(req.getInputStream()); + writer.print(string); + } +} diff --git a/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee10/WEB-INF/appengine-web.xml b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee10/WEB-INF/appengine-web.xml new file mode 100644 index 00000000..7c3e813f --- /dev/null +++ b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee10/WEB-INF/appengine-web.xml @@ -0,0 +1,24 @@ + + + + + java21 + gzip + + + + diff --git a/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee10/WEB-INF/web.xml b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee10/WEB-INF/web.xml new file mode 100644 index 00000000..ca78e1d9 --- /dev/null +++ b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee10/WEB-INF/web.xml @@ -0,0 +1,32 @@ + + + + + + Main + com.google.apphosting.runtime.jetty9.gzipapp.EE10EchoServlet + + + Main + /* + + diff --git a/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee8/WEB-INF/appengine-web.xml b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee8/WEB-INF/appengine-web.xml new file mode 100644 index 00000000..c5e365f0 --- /dev/null +++ b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee8/WEB-INF/appengine-web.xml @@ -0,0 +1,24 @@ + + + + + java21 + gzip + + + + diff --git a/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee8/WEB-INF/web.xml b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee8/WEB-INF/web.xml new file mode 100644 index 00000000..8c47c7d6 --- /dev/null +++ b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/ee8/WEB-INF/web.xml @@ -0,0 +1,32 @@ + + + + + + Main + com.google.apphosting.runtime.jetty9.gzipapp.EE8EchoServlet + + + Main + /* + + diff --git a/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/jetty94/WEB-INF/appengine-web.xml b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/jetty94/WEB-INF/appengine-web.xml new file mode 100644 index 00000000..d2766777 --- /dev/null +++ b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/jetty94/WEB-INF/appengine-web.xml @@ -0,0 +1,25 @@ + + + + + java8 + gzip + true + + + + diff --git a/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/jetty94/WEB-INF/web.xml b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/jetty94/WEB-INF/web.xml new file mode 100644 index 00000000..8c47c7d6 --- /dev/null +++ b/runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/gzipapp/jetty94/WEB-INF/web.xml @@ -0,0 +1,32 @@ + + + + + + Main + com.google.apphosting.runtime.jetty9.gzipapp.EE8EchoServlet + + + Main + /* + + From 274ad9f74390abff89621846856705586ac6ce05 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 13 Nov 2024 16:44:23 +1100 Subject: [PATCH 2/4] optimization for the Jetty 12 JettyRequestAPIData Signed-off-by: Lachlan Roberts --- .../apphosting/runtime/jetty/http/JettyRequestAPIData.java | 6 ++++++ .../apphosting/runtime/jetty9/JettyRequestAPIData.java | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/http/JettyRequestAPIData.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/http/JettyRequestAPIData.java index 8bc17f80..8a3d7962 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/http/JettyRequestAPIData.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/http/JettyRequestAPIData.java @@ -119,6 +119,12 @@ public JettyRequestAPIData( HttpFields.Mutable fields = HttpFields.build(); for (HttpField field : request.getHeaders()) { + // If it has a HttpHeader it is one of the standard headers so won't match any appengine specific header. + if (field.getHeader() != null) { + fields.add(field); + continue; + } + String name = field.getLowerCaseName(); String value = field.getValue(); if (Strings.isNullOrEmpty(value)) { diff --git a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java index cb217fd1..3766b893 100644 --- a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java +++ b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java @@ -135,13 +135,13 @@ public JettyRequestAPIData( continue; } - String lowerCaseName = field.getName().toLowerCase(Locale.ROOT); + String name = field.getName().toLowerCase(Locale.ROOT); String value = field.getValue(); if (Strings.isNullOrEmpty(value)) { continue; } - switch (lowerCaseName) { + switch (name) { case X_APPENGINE_TRUSTED_IP_REQUEST: // If there is a value, then the application is trusted // If the value is IS_TRUSTED, then the user is trusted @@ -243,7 +243,7 @@ public JettyRequestAPIData( break; } - if (passThroughPrivateHeaders || !PRIVATE_APPENGINE_HEADERS.contains(lowerCaseName)) { + if (passThroughPrivateHeaders || !PRIVATE_APPENGINE_HEADERS.contains(name)) { // Only non AppEngine specific headers are passed to the application. fields.add(field); } From fb242505afcec69886b653f5a68b7442bdea14af Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 14 Nov 2024 11:05:33 +1100 Subject: [PATCH 3/4] cleanup of the JettyServletEngineAdapters and JettyHttpProxy for Jetty 9.4 and 12 Signed-off-by: Lachlan Roberts --- .../runtime/jetty/CoreSizeLimitHandler.java | 175 ------------------ .../jetty/JettyServletEngineAdapter.java | 74 ++++---- .../runtime/jetty/proxy/JettyHttpProxy.java | 21 ++- .../runtime/jetty9/JettyHttpProxy.java | 18 +- .../jetty9/JettyServletEngineAdapter.java | 32 ++-- 5 files changed, 83 insertions(+), 237 deletions(-) delete mode 100644 runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/CoreSizeLimitHandler.java diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/CoreSizeLimitHandler.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/CoreSizeLimitHandler.java deleted file mode 100644 index 0d46af3e..00000000 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/CoreSizeLimitHandler.java +++ /dev/null @@ -1,175 +0,0 @@ -/* - * Copyright 2021 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.apphosting.runtime.jetty; - -import java.nio.ByteBuffer; -import org.eclipse.jetty.http.BadMessageException; -import org.eclipse.jetty.http.HttpException; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; -import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.server.Handler; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Response; -import org.eclipse.jetty.util.Callback; - -/** - * A handler that can limit the size of message bodies in requests and responses. - * - *

The optional request and response limits are imposed by checking the {@code Content-Length} - * header or observing the actual bytes seen by the handler. Handler order is important, in as much - * as if this handler is before a the {@link org.eclipse.jetty.server.handler.gzip.GzipHandler}, - * then it will limit compressed sized, if it as after the {@link - * org.eclipse.jetty.server.handler.gzip.GzipHandler} then the limit is applied to uncompressed - * bytes. If a size limit is exceeded then {@link BadMessageException} is thrown with a {@link - * org.eclipse.jetty.http.HttpStatus#PAYLOAD_TOO_LARGE_413} status. - */ -public class CoreSizeLimitHandler extends Handler.Wrapper -{ - private final long _requestLimit; - private final long _responseLimit; - - /** - * @param requestLimit The request body size limit in bytes or -1 for no limit - * @param responseLimit The response body size limit in bytes or -1 for no limit - */ - public CoreSizeLimitHandler(long requestLimit, long responseLimit) - { - _requestLimit = requestLimit; - _responseLimit = responseLimit; - } - - @Override - public boolean handle(Request request, Response response, Callback callback) throws Exception - { - HttpField contentLengthField = request.getHeaders().getField(HttpHeader.CONTENT_LENGTH); - if (contentLengthField != null) - { - long contentLength = contentLengthField.getLongValue(); - if (_requestLimit >= 0 && contentLength > _requestLimit) - { - String s = "Request body is too large: " + contentLength + ">" + _requestLimit; - Response.writeError(request, response, callback, HttpStatus.PAYLOAD_TOO_LARGE_413, s); - return true; - } - } - - SizeLimitRequestWrapper wrappedRequest = new SizeLimitRequestWrapper(request); - SizeLimitResponseWrapper wrappedResponse = new SizeLimitResponseWrapper(wrappedRequest, response); - return super.handle(wrappedRequest, wrappedResponse, callback); - } - - private class SizeLimitRequestWrapper extends Request.Wrapper - { - private long _read = 0; - - public SizeLimitRequestWrapper(Request wrapped) - { - super(wrapped); - } - - @Override - public Content.Chunk read() - { - Content.Chunk chunk = super.read(); - if (chunk == null) - return null; - if (chunk.getFailure() != null) - return chunk; - - // Check request content limit. - ByteBuffer content = chunk.getByteBuffer(); - if (content != null && content.remaining() > 0) - { - _read += content.remaining(); - if (_requestLimit >= 0 && _read > _requestLimit) - { - BadMessageException e = - new BadMessageException( - HttpStatus.PAYLOAD_TOO_LARGE_413, - "Request body is too large: " + _read + ">" + _requestLimit); - getWrapped().fail(e); - return null; - } - } - - return chunk; - } - } - - private class SizeLimitResponseWrapper extends Response.Wrapper - { - private final HttpFields.Mutable _httpFields; - private long _written = 0; - private String failed; - - public SizeLimitResponseWrapper(Request request, Response wrapped) { - super(request, wrapped); - - _httpFields = - new HttpFields.Mutable.Wrapper(wrapped.getHeaders()) { - @Override - public HttpField onAddField(HttpField field) { - if (HttpHeader.CONTENT_LENGTH.is(field.getName())) { - long contentLength = field.getLongValue(); - if (_responseLimit >= 0 && contentLength > _responseLimit) - throw new HttpException.RuntimeException( - HttpStatus.INTERNAL_SERVER_ERROR_500, - "Response body is too large: " + contentLength + ">" + _responseLimit); - } - return super.onAddField(field); - } - }; - } - - @Override - public HttpFields.Mutable getHeaders() { - return _httpFields; - } - - @Override - public void write(boolean last, ByteBuffer content, Callback callback) - { - if (failed != null) { - callback.failed( - new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, failed)); - return; - } - - if (content != null && content.remaining() > 0) - { - if (_responseLimit >= 0 && (_written + content.remaining()) > _responseLimit) - { - failed = - "Response body is too large: " - + _written - + content.remaining() - + ">" - + _responseLimit; - callback.failed( - new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, failed)); - return; - } - _written += content.remaining(); - } - - super.write(last, content, callback); - } - } -} diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java index 160e7197..e57dc371 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java @@ -49,6 +49,7 @@ import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.SizeLimitHandler; import org.eclipse.jetty.util.VirtualThreads; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; @@ -102,6 +103,7 @@ private static AppYaml getAppYaml(ServletEngineAdapter.Config runtimeOptions) { @Override public void start(String serverInfo, ServletEngineAdapter.Config runtimeOptions) { + boolean isHttpConnectorMode = Boolean.getBoolean(HTTP_CONNECTOR_MODE); QueuedThreadPool threadPool = new QueuedThreadPool(MAX_THREAD_POOL_THREADS, MIN_THREAD_POOL_THREADS); // Try to enable virtual threads if requested and on java21: @@ -118,27 +120,45 @@ public InvocationType getInvocationType() { return InvocationType.BLOCKING; } }; - rpcConnector = - new DelegateConnector(server, "RPC") { - @Override - public void run(Runnable runnable) { - // Override this so that it does the initial run in the same thread. - // Currently, we block until completion in serviceRequest() so no point starting new - // thread. - runnable.run(); - } - }; - server.addConnector(rpcConnector); + + // Don't add the RPC Connector if in HttpConnector mode. + if (!isHttpConnectorMode) + { + rpcConnector = + new DelegateConnector(server, "RPC") { + @Override + public void run(Runnable runnable) { + // Override this so that it does the initial run in the same thread. + // Currently, we block until completion in serviceRequest() so no point starting new + // thread. + runnable.run(); + } + }; + + HttpConfiguration httpConfiguration = rpcConnector.getHttpConfiguration(); + httpConfiguration.setSendDateHeader(false); + httpConfiguration.setSendServerVersion(false); + httpConfiguration.setSendXPoweredBy(false); + if (LEGACY_MODE) { + httpConfiguration.setRequestCookieCompliance(CookieCompliance.RFC2965); + httpConfiguration.setResponseCookieCompliance(CookieCompliance.RFC2965); + httpConfiguration.setUriCompliance(UriCompliance.LEGACY); + } + + server.addConnector(rpcConnector); + } + AppVersionHandlerFactory appVersionHandlerFactory = AppVersionHandlerFactory.newInstance(server, serverInfo); appVersionHandler = new AppVersionHandler(appVersionHandlerFactory); - if (!Boolean.getBoolean(IGNORE_RESPONSE_SIZE_LIMIT)) { - CoreSizeLimitHandler sizeLimitHandler = new CoreSizeLimitHandler(-1, MAX_RESPONSE_SIZE); - sizeLimitHandler.setHandler(appVersionHandler); - server.setHandler(sizeLimitHandler); - } else { - server.setHandler(appVersionHandler); + server.setHandler(appVersionHandler); + + // In HttpConnector mode we will combine both SizeLimitHandlers. + boolean ignoreResponseSizeLimit = Boolean.getBoolean(IGNORE_RESPONSE_SIZE_LIMIT); + if (!ignoreResponseSizeLimit && !isHttpConnectorMode) { + server.insertHandler(new SizeLimitHandler(-1, MAX_RESPONSE_SIZE)); } + boolean startJettyHttpProxy = false; if (runtimeOptions.useJettyHttpProxy()) { AppInfoFactory appInfoFactory; @@ -159,14 +179,13 @@ public void run(Runnable runnable) { } catch (Exception e) { throw new IllegalStateException(e); } - if (Boolean.getBoolean(HTTP_CONNECTOR_MODE)) { + if (isHttpConnectorMode) { logger.atInfo().log("Using HTTP_CONNECTOR_MODE to bypass RPC"); - JettyHttpProxy.insertHandlers(server); server.insertHandler( new JettyHttpHandler( runtimeOptions, appVersionHandler.getAppVersion(), appVersionKey, appInfoFactory)); - ServerConnector connector = JettyHttpProxy.newConnector(server, runtimeOptions); - server.addConnector(connector); + JettyHttpProxy.insertHandlers(server, ignoreResponseSizeLimit); + server.addConnector(JettyHttpProxy.newConnector(server, runtimeOptions)); } else { server.setAttribute( "com.google.apphosting.runtime.jetty.appYaml", @@ -197,7 +216,7 @@ public void stop() { } @Override - public void addAppVersion(AppVersion appVersion) throws FileNotFoundException { + public void addAppVersion(AppVersion appVersion) { appVersionHandler.addAppVersion(appVersion); } @@ -239,16 +258,7 @@ public void serviceRequest(UPRequest upRequest, MutableUpResponse upResponse) th } lastAppVersionKey = appVersionKey; } - // TODO: lots of compliance modes to handle. - HttpConfiguration httpConfiguration = rpcConnector.getHttpConfiguration(); - httpConfiguration.setSendDateHeader(false); - httpConfiguration.setSendServerVersion(false); - httpConfiguration.setSendXPoweredBy(false); - if (LEGACY_MODE) { - httpConfiguration.setRequestCookieCompliance(CookieCompliance.RFC2965); - httpConfiguration.setResponseCookieCompliance(CookieCompliance.RFC2965); - httpConfiguration.setUriCompliance(UriCompliance.LEGACY); - } + DelegateRpcExchange rpcExchange = new DelegateRpcExchange(upRequest, upResponse); rpcExchange.setAttribute(AppEngineConstants.APP_VERSION_KEY_REQUEST_ATTR, appVersionKey); rpcExchange.setAttribute(AppEngineConstants.ENVIRONMENT_ATTR, ApiProxy.getCurrentEnvironment()); diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java index f8f03081..24f8bce2 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java @@ -24,7 +24,6 @@ import com.google.apphosting.runtime.ServletEngineAdapter; import com.google.apphosting.runtime.anyrpc.EvaluationRuntimeServerInterface; import com.google.apphosting.runtime.jetty.AppInfoFactory; -import com.google.apphosting.runtime.jetty.CoreSizeLimitHandler; import com.google.apphosting.runtime.jetty.JettyServletEngineAdapter; import com.google.common.base.Ascii; import com.google.common.base.Throwables; @@ -44,9 +43,12 @@ import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.SizeLimitHandler; import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.eclipse.jetty.util.Callback; +import static com.google.apphosting.runtime.AppEngineConstants.IGNORE_RESPONSE_SIZE_LIMIT; + /** * A Jetty web server handling HTTP requests on a given port and forwarding them via gRPC to the * Java8 App Engine runtime implementation. The deployed application is assumed to be located in a @@ -68,6 +70,7 @@ public class JettyHttpProxy { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); private static final long MAX_REQUEST_SIZE = 32 * 1024 * 1024; + private static final long MAX_RESPONSE_SIZE = 32 * 1024 * 1024; /** * Based on the adapter configuration, this will start a new Jetty server in charge of proxying @@ -108,22 +111,26 @@ public static ServerConnector newConnector( return connector; } - public static void insertHandlers(Server server) { - CoreSizeLimitHandler sizeLimitHandler = new CoreSizeLimitHandler(MAX_REQUEST_SIZE, -1); - sizeLimitHandler.setHandler(server.getHandler()); + public static void insertHandlers(Server server, boolean ignoreResponseSizeLimit) { + + long responseLimit = -1; + if (!ignoreResponseSizeLimit) { + responseLimit = MAX_RESPONSE_SIZE; + } + SizeLimitHandler sizeLimitHandler = new SizeLimitHandler(MAX_REQUEST_SIZE, responseLimit); + server.insertHandler(sizeLimitHandler); GzipHandler gzip = new GzipHandler(); gzip.setInflateBufferSize(8 * 1024); - gzip.setHandler(sizeLimitHandler); gzip.setIncludedMethods(); // Include all methods for the GzipHandler. - server.setHandler(gzip); + server.insertHandler(gzip); } public static Server newServer( ServletEngineAdapter.Config runtimeOptions, ForwardingHandler forwardingHandler) { Server server = new Server(); server.setHandler(forwardingHandler); - insertHandlers(server); + insertHandlers(server, true); ServerConnector connector = newConnector(server, runtimeOptions); server.addConnector(connector); diff --git a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyHttpProxy.java b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyHttpProxy.java index ec75a168..cb1768b0 100644 --- a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyHttpProxy.java +++ b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyHttpProxy.java @@ -66,6 +66,7 @@ public class JettyHttpProxy { private static final String JETTY_LOG_CLASS = "org.eclipse.jetty.util.log.class"; private static final String JETTY_STDERRLOG = "org.eclipse.jetty.util.log.StdErrLog"; private static final long MAX_REQUEST_SIZE = 32 * 1024 * 1024; + private static final long MAX_RESPONSE_SIZE = 32 * 1024 * 1024; /** * Based on the adapter configuration, this will start a new Jetty server in charge of proxying @@ -104,23 +105,26 @@ public static ServerConnector newConnector( return connector; } - public static void insertHandlers(Server server) { - SizeLimitHandler sizeLimitHandler = new SizeLimitHandler(MAX_REQUEST_SIZE, -1); - sizeLimitHandler.setHandler(server.getHandler()); + public static void insertHandlers(Server server, boolean ignoreResponseSizeLimit) { + + long responseLimit = -1; + if (!ignoreResponseSizeLimit) { + responseLimit = MAX_RESPONSE_SIZE; + } + SizeLimitHandler sizeLimitHandler = new SizeLimitHandler(MAX_REQUEST_SIZE, responseLimit); + server.insertHandler(sizeLimitHandler); GzipHandler gzip = new GzipHandler(); gzip.setInflateBufferSize(8 * 1024); - gzip.setHandler(sizeLimitHandler); - gzip.setExcludedAgentPatterns(); gzip.setIncludedMethods(); // Include all methods for the GzipHandler. - server.setHandler(gzip); + server.insertHandler(gzip); } public static Server newServer( ServletEngineAdapter.Config runtimeOptions, ForwardingHandler handler) { Server server = new Server(); server.setHandler(handler); - insertHandlers(server); + insertHandlers(server, true); ServerConnector connector = newConnector(server, runtimeOptions); server.addConnector(connector); diff --git a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java index 86644f4e..6fc7e1e8 100644 --- a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java +++ b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java @@ -42,9 +42,7 @@ import java.util.Objects; import java.util.Optional; import javax.servlet.ServletException; -import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.SizeLimitHandler; import org.eclipse.jetty.util.thread.QueuedThreadPool; @@ -104,21 +102,24 @@ private static AppYaml getAppYaml(ServletEngineAdapter.Config runtimeOptions) { @Override public void start(String serverInfo, ServletEngineAdapter.Config runtimeOptions) { + boolean isHttpConnectorMode = Boolean.getBoolean(HTTP_CONNECTOR_MODE); server = new Server(new QueuedThreadPool(MAX_THREAD_POOL_THREADS, MIN_THREAD_POOL_THREADS)); - rpcConnector = new RpcConnector(server); - server.setConnectors(new Connector[] {rpcConnector}); + + if (!isHttpConnectorMode) { + rpcConnector = new RpcConnector(server); + server.addConnector(rpcConnector); + } + AppVersionHandlerFactory appVersionHandlerFactory = new AppVersionHandlerFactory( server, serverInfo, contextFactory, /* useJettyErrorPageHandler= */ false); appVersionHandlerMap = new AppVersionHandlerMap(appVersionHandlerFactory); + server.setHandler(appVersionHandlerMap); - if (!Objects.equals(System.getenv("GAE_RUNTIME"), "java8") - && !Boolean.getBoolean(IGNORE_RESPONSE_SIZE_LIMIT)) { - SizeLimitHandler sizeLimitHandler = new SizeLimitHandler(-1, MAX_RESPONSE_SIZE); - sizeLimitHandler.setHandler(appVersionHandlerMap); - server.setHandler(sizeLimitHandler); - } else { - server.setHandler(appVersionHandlerMap); + boolean ignoreResponseSizeLimit = Objects.equals(System.getenv("GAE_RUNTIME"), "java8") + || Boolean.getBoolean(IGNORE_RESPONSE_SIZE_LIMIT); + if (!ignoreResponseSizeLimit && !isHttpConnectorMode) { + server.insertHandler(new SizeLimitHandler(-1, MAX_RESPONSE_SIZE)); } try { @@ -137,15 +138,14 @@ public void start(String serverInfo, ServletEngineAdapter.Config runtimeOptions) evaluationRuntimeServerInterface.addAppVersion(context, appinfo); EmptyMessage unused = context.getResponse(); - if (Boolean.getBoolean(HTTP_CONNECTOR_MODE)) { + if (isHttpConnectorMode) { logger.atInfo().log("Using HTTP_CONNECTOR_MODE to bypass RPC"); appVersionKey = AppVersionKey.fromAppInfo(appinfo); AppVersion appVersion = appVersionHandlerMap.getAppVersion(appVersionKey); server.insertHandler( new JettyHttpHandler(runtimeOptions, appVersion, appVersionKey, appInfoFactory)); - JettyHttpProxy.insertHandlers(server); - ServerConnector connector = JettyHttpProxy.newConnector(server, runtimeOptions); - server.addConnector(connector); + JettyHttpProxy.insertHandlers(server, ignoreResponseSizeLimit); + server.addConnector(JettyHttpProxy.newConnector(server, runtimeOptions)); } else { server.setAttribute( "com.google.apphosting.runtime.jetty9.appYaml", @@ -184,7 +184,7 @@ public void stop() { } @Override - public void addAppVersion(AppVersion appVersion) throws FileNotFoundException { + public void addAppVersion(AppVersion appVersion) { appVersionHandlerMap.addAppVersion(appVersion); } From 8e3083f915ef75f56fdb45f63c6f0cf7d042ac06 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 15 Nov 2024 13:09:20 +1100 Subject: [PATCH 4/4] reformat JettyServletEngineAdapter with google style Signed-off-by: Lachlan Roberts --- .../apphosting/runtime/jetty/JettyServletEngineAdapter.java | 3 +-- .../apphosting/runtime/jetty9/JettyServletEngineAdapter.java | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java index e57dc371..129da7f3 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java @@ -122,8 +122,7 @@ public InvocationType getInvocationType() { }; // Don't add the RPC Connector if in HttpConnector mode. - if (!isHttpConnectorMode) - { + if (!isHttpConnectorMode) { rpcConnector = new DelegateConnector(server, "RPC") { @Override diff --git a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java index 6fc7e1e8..41e2e25e 100644 --- a/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java +++ b/runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyServletEngineAdapter.java @@ -116,7 +116,8 @@ public void start(String serverInfo, ServletEngineAdapter.Config runtimeOptions) appVersionHandlerMap = new AppVersionHandlerMap(appVersionHandlerFactory); server.setHandler(appVersionHandlerMap); - boolean ignoreResponseSizeLimit = Objects.equals(System.getenv("GAE_RUNTIME"), "java8") + boolean ignoreResponseSizeLimit = + Objects.equals(System.getenv("GAE_RUNTIME"), "java8") || Boolean.getBoolean(IGNORE_RESPONSE_SIZE_LIMIT); if (!ignoreResponseSizeLimit && !isHttpConnectorMode) { server.insertHandler(new SizeLimitHandler(-1, MAX_RESPONSE_SIZE));