Fix issues with CookieCompliance on Jetty 9.4 HttpConnector mode. by lachlan-roberts · Pull Request #418 · GoogleCloudPlatform/appengine-java-standard

lachlan-roberts · 2025-10-01T14:35:45Z

The Cookie Compliance was not set to RFC2965 when LEGACY_MODE was true on Jetty 9.4 runtimes when useHttpConnector=true.

This would have been shown as a failure in CookieComplianceTest if was parameterized over all the possible runtime options.

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

gemini-code-assist · 2025-10-01T14:36:08Z

Summary of Changes

Hello @lachlan-roberts, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a specific configuration oversight in Jetty 9.4 where cookie and multipart form data compliance settings were not correctly applied under certain legacy and HTTP connector usage scenarios. The changes ensure that the system adheres to the appropriate RFC2965 standard for cookies and legacy compliance for multipart forms. A significant part of this update also involves enhancing the test suite by parameterizing the cookie compliance tests, making them more robust and capable of validating behavior across a broader range of runtime environments.

Highlights

Cookie Compliance Fix: Addressed an issue where CookieCompliance was not correctly set to RFC2965 for Jetty 9.4 runtimes when LEGACY_MODE was true and useHttpConnector was enabled. This ensures proper handling of cookies under these specific conditions.
Multipart Form Data Compliance: Configured MultiPartFormDataCompliance to LEGACY when LEGACY_MODE is active and useHttpConnector is true, aligning with the intended behavior for legacy environments.
Parameterized Testing: Refactored CookieComplianceTest to be parameterized, allowing it to run across various runtime options (including different Jakarta/Javax versions and useHttpConnector settings) to improve test coverage and prevent similar regressions.
Test Application Structure: Introduced separate test application directories (jakarta and javax) for cookie compliance tests, enabling distinct configurations and servlet implementations for different Java EE versions.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request addresses an issue with cookie compliance in Jetty 9.4's HttpConnector mode by correctly setting the compliance level to RFC2965 when in legacy mode. The changes look correct and are well-supported by an expansion of the test suite to be parameterized, which will help prevent future regressions across different runtime configurations.

I have a couple of minor suggestions regarding coding style in JettyHttpProxy.java to improve consistency and adherence to common Java conventions.

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

ludoch · 2025-10-01T14:53:12Z

 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.awaitility.Awaitility.await;

+import com.google.appengine.repackaged.com.google.protobuf.ByteString;


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.

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

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

I will revert

ludoch

Tryin on google3 now.

ludoch · 2025-10-01T15:42:06Z


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


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

appengine-java-standard/.github/workflows/maven.yml

Line 42 in fdeccc2

maven_profile: "-Pjdk25"

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.

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

ludoch · 2025-10-01T23:07:17Z

Locally, you have only one JDK, and you compile all with whatever is default in pom.
On gh actions, we change the jdk target byte code via a profile and we use different jdks.
I also have hard time to understand but when I removed the extra check with "9.4" it works.

PiperOrigin-RevId: 813962202 Change-Id: Icd014d5128b4bdbf2c96a7758b57b5d9648e3ede

ludoch · 2025-10-01T23:40:32Z

Thanks!, merged now with newly added internal files for internal tests

lachlan-roberts added 2 commits October 2, 2025 00:09

Fix issues with CookieCompliance on Jetty 9.4 HttpConnector mode.

f71ace0

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

Fix issues with CookieCompliance on Jetty 9.4 HttpConnector mode.

3a4a62d

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

lachlan-roberts requested review from ludoch and maigovannon October 1, 2025 14:35

gemini-code-assist Bot reviewed Oct 1, 2025

View reviewed changes

Comment thread ...e/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyHttpProxy.java Outdated

Comment thread ...e/runtime_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyHttpProxy.java Outdated

Fix bad formatting

fc81e91

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

ludoch reviewed Oct 1, 2025

View reviewed changes

ludoch approved these changes Oct 1, 2025

View reviewed changes

ludoch reviewed Oct 1, 2025

View reviewed changes

remove jetty9.4 constraint

8d302a5

ludoch approved these changes Oct 1, 2025

View reviewed changes

ludoch added the ready to pull label Oct 1, 2025

Fixes for import ordering

368805b

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

copybara-service Bot pushed a commit that referenced this pull request Oct 1, 2025

Merge pull request #418 from GoogleCloudPlatform:CookieComplianceFixes

874ba62

PiperOrigin-RevId: 813962202 Change-Id: Icd014d5128b4bdbf2c96a7758b57b5d9648e3ede

ludoch closed this Oct 1, 2025

lachlan-roberts deleted the CookieComplianceFixes branch October 2, 2025 05:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issues with CookieCompliance on Jetty 9.4 HttpConnector mode.#418

Fix issues with CookieCompliance on Jetty 9.4 HttpConnector mode.#418
lachlan-roberts wants to merge 5 commits intomainfrom
CookieComplianceFixes

lachlan-roberts commented Oct 1, 2025

Uh oh!

gemini-code-assist Bot commented Oct 1, 2025

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

ludoch Oct 1, 2025

Uh oh!

ludoch Oct 1, 2025

Uh oh!

lachlan-roberts Oct 1, 2025

Uh oh!

ludoch left a comment

Uh oh!

ludoch Oct 1, 2025 •

edited

Loading

Uh oh!

lachlan-roberts Oct 1, 2025

Uh oh!

ludoch commented Oct 1, 2025

Uh oh!

ludoch commented Oct 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lachlan-roberts commented Oct 1, 2025

Uh oh!

gemini-code-assist Bot commented Oct 1, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

ludoch Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ludoch Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

lachlan-roberts Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ludoch left a comment

Choose a reason for hiding this comment

Uh oh!

ludoch Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lachlan-roberts Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ludoch commented Oct 1, 2025

Uh oh!

ludoch commented Oct 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ludoch Oct 1, 2025 •

edited

Loading