Merge pull request #309 from GoogleCloudPlatform:cleanupOfJettyServle…

…tEngineAdaptors PiperOrigin-RevId: 697802010 Change-Id: I1f6149e6fd89a1ab5d1ce75b9ae416b8f0fb4faf
GoogleCloudPlatform · Nov 19, 2024 · 8c7d268 · 8c7d268
2 parents 7783c8b + 98fb566
commit 8c7d268
Show file tree

Hide file tree

Showing 17 changed files with 511 additions and 243 deletions.
diff --git a/..._impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/CoreSizeLimitHandler.java b/..._impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/CoreSizeLimitHandler.java
diff --git a/..._jetty12/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java b/..._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,44 @@ 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 +178,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 +215,7 @@ public void stop() {
   }
 
   @Override
-  public void addAppVersion(AppVersion appVersion) throws FileNotFoundException {
+  public void addAppVersion(AppVersion appVersion) {
     appVersionHandler.addAppVersion(appVersion);
   }
 
@@ -239,16 +257,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/...l_jetty12/src/main/java/com/google/apphosting/runtime/jetty/http/JettyRequestAPIData.java b/...l_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/..._impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java b/..._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);