Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,12 +36,9 @@
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.Request;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.*;
Comment thread
lachlan-roberts marked this conversation as resolved.
Outdated
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.server.handler.SizeLimitHandler;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
Expand Down Expand Up @@ -104,6 +102,13 @@ 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);
}
Comment thread
lachlan-roberts marked this conversation as resolved.
Outdated

return connector;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
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 org.apache.http.Header;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
Expand All @@ -28,28 +30,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 || "EE6".equals(jakartaVersion));

String app = "com/google/apphosting/runtime/jetty9/cookiecomplianceapp/";
if (isJakarta()) {
app = app + "jakarta";
} else {
app = app + "javax";
}
copyAppToDir(app, temp.getRoot().toPath());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a pb when pushing to google3 where we use un repackaged. I will try and might have to clone in a CL to fix, and then push back to gh.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or better, keep them in the same place, so our automation back to google3 will be happy.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my intellij has somehow lost the correct import ordering in the codestyle settings, so its being auto formatted badly

I will revert

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;
Expand All @@ -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;
Expand Down Expand Up @@ -171,7 +171,7 @@ public static List<Object[]> allVersions() {
System.out.println("javaVersionForTest " + javaVersionForTest);

return allVersions.stream()
.filter(v -> v[0].toString().equals(javaVersionForTest))
.filter(v -> v[0].toString().equals(javaVersionForTest) || v[1].equals("9.4"))
Copy link
Copy Markdown
Collaborator

@ludoch ludoch Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our GH actions fail for jdk25 and 26 ea as they use bytecode for jsdk25, and I assume jetty 9.4 runtime does not work for these runtimes, so we need to filter 9.4 with excluding 25 and 26-ea See https://github.com/GoogleCloudPlatform/appengine-java-standard/actions/runs/18166264666
and jdk25 profile used in

maven_profile: "-Pjdk25"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't expect any issues using JDK 25 with Jetty 9.4, so I don't know why this would be failing.

Seemed to be some JVM crash in GuestBookTest, but I couldn't see why, and didn't happen for me locally.

.collect(toImmutableList());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading