Merge pull request #418 from GoogleCloudPlatform:CookieComplianceFixes

PiperOrigin-RevId: 813962202
Change-Id: Icd014d5128b4bdbf2c96a7758b57b5d9648e3ede

Author: gae-java-bot

runtime/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyHttpProxy.java

@@ -16,6 +16,7 @@
 
 package com.google.apphosting.runtime.jetty9;
 
+import static com.google.apphosting.runtime.AppEngineConstants.HTTP_CONNECTOR_MODE;
 import static com.google.apphosting.runtime.AppEngineConstants.LEGACY_MODE;
 
 import com.google.apphosting.base.protos.AppLogsPb;
@@ -35,9 +36,11 @@
 import java.util.logging.Level;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jetty.http.CookieCompliance;
 import org.eclipse.jetty.http.HttpCompliance;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.HttpConnectionFactory;
+import org.eclipse.jetty.server.MultiPartFormDataCompliance;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
@@ -94,8 +97,7 @@ public static ServerConnector newConnector(
     connector.setPort(runtimeOptions.jettyHttpAddress().getPort());
 
     HttpConnectionFactory factory = connector.getConnectionFactory(HttpConnectionFactory.class);
-    factory.setHttpCompliance(
-        LEGACY_MODE ? HttpCompliance.RFC7230_LEGACY : HttpCompliance.RFC7230);
+    factory.setHttpCompliance(LEGACY_MODE ? HttpCompliance.RFC7230_LEGACY : HttpCompliance.RFC7230);
 
     HttpConfiguration config = factory.getHttpConfiguration();
     config.setRequestHeaderSize(runtimeOptions.jettyRequestHeaderSize());
@@ -104,6 +106,12 @@ public static ServerConnector newConnector(
     config.setSendServerVersion(false);
     config.setSendXPoweredBy(false);
 
+    if (LEGACY_MODE && Boolean.getBoolean(HTTP_CONNECTOR_MODE)) {
+      config.setRequestCookieCompliance(CookieCompliance.RFC2965);
+      config.setResponseCookieCompliance(CookieCompliance.RFC2965);
+      config.setMultiPartFormDataCompliance(MultiPartFormDataCompliance.LEGACY);
+    }
+
     return connector;
   }
 
@@ -211,17 +219,12 @@ UPResponse getUpResponse(UPRequest upRequest) throws ExecutionException, Interru
   }
 
   private static Level toJavaLevel(long level) {
-    switch (Ints.saturatedCast(level)) {
-      case 0:
-        return Level.FINE;
-      case 1:
-        return Level.INFO;
-      case 3:
-      case 4:
-        return Level.SEVERE;
-      default:
-        return Level.WARNING;
-    }
+    return switch (Ints.saturatedCast(level)) {
+      case 0 -> Level.FINE;
+      case 1 -> Level.INFO;
+      case 3, 4 -> Level.SEVERE;
+      default -> Level.WARNING;
+    };
   }
 
   private JettyHttpProxy() {}

runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/CookieComplianceTest.java

@@ -17,7 +17,10 @@
 package com.google.apphosting.runtime.jetty9;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assume.assumeTrue;
 
+import java.util.List;
+import java.util.Objects;
 import org.apache.http.Header;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
@@ -28,28 +31,40 @@
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.runners.Parameterized;
 
-@RunWith(JUnit4.class)
+@RunWith(Parameterized.class)
 public class CookieComplianceTest extends JavaRuntimeViaHttpBase {
 
-  // This is set in the app appengine-web.xml file
+  // This is set in the app appengine-web.xml file.
   static {
     System.setProperty("com.google.apphosting.runtime.jetty94.LEGACY_MODE", "true");
   }
 
-  public CookieComplianceTest() {
-    //Test also running in google3, so we limit to jetty 9.4 for now.
-    // TODO(ludo): Enable for other versions once we remove internal jetty94 dependency.
-    // TODO(ludo): http connector true: fails, but http connector false: pass
-    super("java17", "9.4", "EE6", false);
+    @Parameterized.Parameters
+    public static List<Object[]> version() {
+        return allVersions();
+    }
+
+  public CookieComplianceTest(String runtimeVersion, String jettyVersion, String version, boolean useHttpConnector) {
+      super(runtimeVersion, jettyVersion, version, useHttpConnector);
   }
 
   @Rule public TemporaryFolder temp = new TemporaryFolder();
 
   @Before
   public void copyAppToTemp() throws Exception {
-    copyAppToDir("cookiecomplianceapp", temp.getRoot().toPath());
+      // Internal testing is limited to Jetty 9.4 EE6 for now.
+      boolean internal = Boolean.getBoolean("test.running.internally");
+    assumeTrue(!internal || Objects.equals(jakartaVersion, "EE6"));
+
+      String app = "com/google/apphosting/runtime/jetty9/cookiecomplianceapp/";
+      if (isJakarta()) {
+          app = app + "jakarta";
+      } else {
+          app = app + "javax";
+      }
+      copyAppToDir(app, temp.getRoot().toPath());
   }
 
   @Test

runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java

@@ -31,6 +31,10 @@
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.awaitility.Awaitility.await;
 
+import com.google.appengine.repackaged.com.google.protobuf.ByteString;
+import com.google.appengine.repackaged.com.google.protobuf.ExtensionRegistry;
+import com.google.appengine.repackaged.com.google.protobuf.InvalidProtocolBufferException;
+import com.google.appengine.repackaged.com.google.protobuf.UninitializedMessageException;
 import com.google.apphosting.base.protos.api.RemoteApiPb;
 import com.google.apphosting.testing.PortPicker;
 import com.google.auto.value.AutoValue;
@@ -45,10 +49,6 @@
 import com.google.common.reflect.Reflection;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.errorprone.annotations.ForOverride;
-import com.google.appengine.repackaged.com.google.protobuf.ByteString;
-import com.google.appengine.repackaged.com.google.protobuf.ExtensionRegistry;
-import com.google.appengine.repackaged.com.google.protobuf.InvalidProtocolBufferException;
-import com.google.appengine.repackaged.com.google.protobuf.UninitializedMessageException;
 import com.sun.net.httpserver.HttpExchange;
 import com.sun.net.httpserver.HttpServer;
 import java.io.BufferedReader;

runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/JakartaCookieTestServlet.java

@@ -0,0 +1,37 @@
+/*
+ * 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.cookiecomplianceapp;
+
+import jakarta.servlet.annotation.WebServlet;
+import jakarta.servlet.http.Cookie;
+import jakarta.servlet.http.HttpServlet;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.io.PrintWriter;
+
+/** This servlet sets a cookie which is illegal to be set under the rules of RFC6265. */
+@WebServlet(urlPatterns = "/*")
+public class JakartaCookieTestServlet extends HttpServlet {
+  @Override
+  protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+    PrintWriter writer = resp.getWriter();
+    resp.setContentType("text/plain");
+    writer.print("cookieTestServletContent");
+    resp.addCookie(new Cookie("invalidRFC6265Cookie", "value\"1234"));
+  }
+}

…okiecomplianceapp/CookieTestServlet.java …omplianceapp/JavaxCookieTestServlet.javaruntime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/CookieTestServlet.java renamed to runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/JavaxCookieTestServlet.java runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/CookieTestServlet.java renamed to runtime/testapps/src/main/java/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/JavaxCookieTestServlet.java

@@ -26,7 +26,7 @@
 
 /** This servlet sets a cookie which is illegal to be set under the rules of RFC6265. */
 @WebServlet(urlPatterns = "/*")
-public class CookieTestServlet extends HttpServlet {
+public class JavaxCookieTestServlet extends HttpServlet {
   @Override
   protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
     PrintWriter writer = resp.getWriter();

…ecomplianceapp/WEB-INF/appengine-web.xml …nceapp/jakarta/WEB-INF/appengine-web.xmlruntime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/WEB-INF/appengine-web.xml renamed to runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/jakarta/WEB-INF/appengine-web.xml runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/WEB-INF/appengine-web.xml renamed to runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/jakarta/WEB-INF/appengine-web.xml

@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+ 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.
+-->
+
+<web-app xmlns="http://java.sun.com/xml/ns/javaee"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="3.1"
+    xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_1.xsd">
+  <servlet>
+    <servlet-name>CookieTestServlet</servlet-name>
+    <servlet-class>com.google.apphosting.runtime.jetty9.cookiecomplianceapp.JakartaCookieTestServlet</servlet-class>
+  </servlet>
+  <servlet-mapping>
+    <servlet-name>CookieTestServlet</servlet-name>
+    <url-pattern>/*</url-pattern>
+  </servlet-mapping>
+</web-app>

runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/jakarta/WEB-INF/web.xml

@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+ 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.
+-->
+
+<appengine-web-app xmlns="http://appengine.google.com/ns/1.0">
+  <runtime>java17</runtime>
+  <application>cookiecomplianceapp</application>
+  <version>1</version>
+  <system-properties>
+    <property name="com.google.apphosting.runtime.jetty94.LEGACY_MODE" value="true" />
+  </system-properties>
+</appengine-web-app>

runtime/testapps/src/main/resources/com/google/apphosting/runtime/jetty9/cookiecomplianceapp/javax/WEB-INF/appengine-web.xml

@@ -20,7 +20,7 @@
     xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_1.xsd">
   <servlet>
     <servlet-name>CookieTestServlet</servlet-name>
-    <servlet-class>com.google.apphosting.runtime.jetty9.cookiecomplianceapp.CookieTestServlet</servlet-class>
+    <servlet-class>com.google.apphosting.runtime.jetty9.cookiecomplianceapp.JavaxCookieTestServlet</servlet-class>
   </servlet>
   <servlet-mapping>
     <servlet-name>CookieTestServlet</servlet-name>