Skip to content

Commit f71ace0

Browse files
lachlan-robertslachlan-roberts
committed
Fix issues with CookieCompliance on Jetty 9.4 HttpConnector mode.
Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
1 parent fdeccc2 commit f71ace0

8 files changed

Lines changed: 146 additions & 15 deletions

File tree

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.apphosting.runtime.jetty9;
1818

19+
import static com.google.apphosting.runtime.AppEngineConstants.HTTP_CONNECTOR_MODE;
1920
import static com.google.apphosting.runtime.AppEngineConstants.LEGACY_MODE;
2021

2122
import com.google.apphosting.base.protos.AppLogsPb;
@@ -35,12 +36,9 @@
3536
import java.util.logging.Level;
3637
import javax.servlet.http.HttpServletRequest;
3738
import javax.servlet.http.HttpServletResponse;
39+
import org.eclipse.jetty.http.CookieCompliance;
3840
import org.eclipse.jetty.http.HttpCompliance;
39-
import org.eclipse.jetty.server.HttpConfiguration;
40-
import org.eclipse.jetty.server.HttpConnectionFactory;
41-
import org.eclipse.jetty.server.Request;
42-
import org.eclipse.jetty.server.Server;
43-
import org.eclipse.jetty.server.ServerConnector;
41+
import org.eclipse.jetty.server.*;
4442
import org.eclipse.jetty.server.handler.AbstractHandler;
4543
import org.eclipse.jetty.server.handler.SizeLimitHandler;
4644
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
@@ -104,6 +102,13 @@ public static ServerConnector newConnector(
104102
config.setSendServerVersion(false);
105103
config.setSendXPoweredBy(false);
106104

105+
if (LEGACY_MODE && Boolean.getBoolean(HTTP_CONNECTOR_MODE))
106+
{
107+
config.setRequestCookieCompliance(CookieCompliance.RFC2965);
108+
config.setResponseCookieCompliance(CookieCompliance.RFC2965);
109+
config.setMultiPartFormDataCompliance(MultiPartFormDataCompliance.LEGACY);
110+
}
111+
107112
return connector;
108113
}
109114

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

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717
package com.google.apphosting.runtime.jetty9;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static org.junit.Assume.assumeTrue;
2021

22+
import java.util.Arrays;
23+
import java.util.List;
2124
import org.apache.http.Header;
2225
import org.apache.http.HttpResponse;
2326
import org.apache.http.client.HttpClient;
@@ -28,28 +31,60 @@
2831
import org.junit.Test;
2932
import org.junit.rules.TemporaryFolder;
3033
import org.junit.runner.RunWith;
31-
import org.junit.runners.JUnit4;
34+
import org.junit.runners.Parameterized;
3235

33-
@RunWith(JUnit4.class)
36+
@RunWith(Parameterized.class)
3437
public class CookieComplianceTest extends JavaRuntimeViaHttpBase {
3538

3639
// This is set in the app appengine-web.xml file
3740
static {
3841
System.setProperty("com.google.apphosting.runtime.jetty94.LEGACY_MODE", "true");
3942
}
4043

41-
public CookieComplianceTest() {
42-
//Test also running in google3, so we limit to jetty 9.4 for now.
43-
// TODO(ludo): Enable for other versions once we remove internal jetty94 dependency.
44-
// TODO(ludo): http connector true: fails, but http connector false: pass
45-
super("java17", "9.4", "EE6", false);
44+
@Parameterized.Parameters
45+
public static List<Object[]> version() {
46+
return Arrays.asList(
47+
new Object[][] {
48+
{"java17", "9.4", "EE6", false},
49+
{"java17", "12.0", "EE8", false},
50+
{"java17", "12.0", "EE10", false},
51+
{"java17", "12.1", "EE11", false},
52+
{"java21", "12.0", "EE8", false},
53+
{"java21", "12.0", "EE10", false},
54+
{"java21", "12.1", "EE11", false},
55+
{"java25", "12.1", "EE8", false},
56+
{"java25", "12.1", "EE11", false},
57+
{"java17", "9.4", "EE6", true},
58+
{"java17", "12.0", "EE8", true},
59+
{"java17", "12.0", "EE10", true},
60+
{"java17", "12.1", "EE11", true},
61+
{"java21", "12.0", "EE8", true},
62+
{"java21", "12.0", "EE10", true},
63+
{"java21", "12.1", "EE11", true},
64+
{"java25", "12.1", "EE8", true},
65+
{"java25", "12.1", "EE11", true},
66+
});
67+
}
68+
69+
public CookieComplianceTest(String runtimeVersion, String jettyVersion, String version, boolean useHttpConnector) {
70+
super(runtimeVersion, jettyVersion, version, useHttpConnector);
4671
}
4772

4873
@Rule public TemporaryFolder temp = new TemporaryFolder();
4974

5075
@Before
5176
public void copyAppToTemp() throws Exception {
52-
copyAppToDir("cookiecomplianceapp", temp.getRoot().toPath());
77+
// Internal testing is limited to Jetty 9.4 EE6 for now.
78+
boolean internal = Boolean.getBoolean("test.running.internally");
79+
assumeTrue(!internal || "EE6".equals(jakartaVersion));
80+
81+
String app = "com/google/apphosting/runtime/jetty9/cookiecomplianceapp/";
82+
if (isJakarta()) {
83+
app = app + "jakarta";
84+
} else {
85+
app = app + "javax";
86+
}
87+
copyAppToDir(app, temp.getRoot().toPath());
5388
}
5489

5590
@Test

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2021 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.apphosting.runtime.jetty9.cookiecomplianceapp;
18+
19+
import jakarta.servlet.annotation.WebServlet;
20+
import jakarta.servlet.http.Cookie;
21+
import jakarta.servlet.http.HttpServlet;
22+
import jakarta.servlet.http.HttpServletRequest;
23+
import jakarta.servlet.http.HttpServletResponse;
24+
import java.io.IOException;
25+
import java.io.PrintWriter;
26+
27+
/** This servlet sets a cookie which is illegal to be set under the rules of RFC6265. */
28+
@WebServlet(urlPatterns = "/*")
29+
public class JakartaCookieTestServlet extends HttpServlet {
30+
@Override
31+
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
32+
PrintWriter writer = resp.getWriter();
33+
resp.setContentType("text/plain");
34+
writer.print("cookieTestServletContent");
35+
resp.addCookie(new Cookie("invalidRFC6265Cookie", "value\"1234"));
36+
}
37+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

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

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

File renamed without changes.

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<!--
3+
Copyright 2021 Google LLC
4+
5+
Licensed under the Apache License, Version 2.0 (the "License");
6+
you may not use this file except in compliance with the License.
7+
You may obtain a copy of the License at
8+
9+
https://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
-->
17+
18+
<web-app xmlns="http://java.sun.com/xml/ns/javaee"
19+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="3.1"
20+
xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_1.xsd">
21+
<servlet>
22+
<servlet-name>CookieTestServlet</servlet-name>
23+
<servlet-class>com.google.apphosting.runtime.jetty9.cookiecomplianceapp.JakartaCookieTestServlet</servlet-class>
24+
</servlet>
25+
<servlet-mapping>
26+
<servlet-name>CookieTestServlet</servlet-name>
27+
<url-pattern>/*</url-pattern>
28+
</servlet-mapping>
29+
</web-app>

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<!--
3+
Copyright 2021 Google LLC
4+
5+
Licensed under the Apache License, Version 2.0 (the "License");
6+
you may not use this file except in compliance with the License.
7+
You may obtain a copy of the License at
8+
9+
https://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
-->
17+
18+
<appengine-web-app xmlns="http://appengine.google.com/ns/1.0">
19+
<runtime>java17</runtime>
20+
<application>cookiecomplianceapp</application>
21+
<version>1</version>
22+
<system-properties>
23+
<property name="com.google.apphosting.runtime.jetty94.LEGACY_MODE" value="true" />
24+
</system-properties>
25+
</appengine-web-app>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_1.xsd">
2121
<servlet>
2222
<servlet-name>CookieTestServlet</servlet-name>
23-
<servlet-class>com.google.apphosting.runtime.jetty9.cookiecomplianceapp.CookieTestServlet</servlet-class>
23+
<servlet-class>com.google.apphosting.runtime.jetty9.cookiecomplianceapp.JavaxCookieTestServlet</servlet-class>
2424
</servlet>
2525
<servlet-mapping>
2626
<servlet-name>CookieTestServlet</servlet-name>

0 commit comments

Comments
 (0)