-
Notifications
You must be signed in to change notification settings - Fork 63
Fix issues with CookieCompliance on Jetty 9.4 HttpConnector mode. #418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
f71ace0
3a4a62d
fc81e91
8d302a5
368805b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -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; | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||
|
|
@@ -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; | ||||
|
|
@@ -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")) | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||
| .collect(toImmutableList()); | ||||
| } | ||||
|
|
||||
|
|
||||
| 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 |
|---|---|---|
| @@ -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> |
Uh oh!
There was an error while loading. Please reload this page.