Skip to content

Commit cddf142

Browse files
markt-asfmarkt-asf
committed
Ensure messages are received in the correct order
Fix issue #900 and improve ordering checks generally. Use a single expected response so that ordering of the individual components is checked. Avoid a test hang if the expected response is not provided. Remove unused and/or unnecessary code Track filter invocation order using a header (headers must be written before switching protocols) The second chunk of the upgraded message ("world") may be received with "Hello" or separately so don't look for "onDataAvailable" before it. Fix some consistency issues in the log messages Remove unused (and unnecessary) code
1 parent 7b08197 commit cddf142

4 files changed

Lines changed: 93 additions & 51 deletions

File tree

‎tck/tck-runtime/src/main/java/servlet/tck/api/jakarta_servlet_http/httpupgradehandler/HttpUpgradeHandlerTests.java‎

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public void setupServletName() throws Exception {
5151
public static WebArchive getTestArchive() throws Exception {
5252
return ShrinkWrap.create(WebArchive.class, "servlet_jsh_upgradehandler_web.war")
5353
.addAsLibraries(CommonServlets.getCommonServletsArchive())
54-
.addClasses(TCKHttpUpgradeHandler.class, TCKReadListener.class, TestServlet.class);
54+
.addClasses(TCKHttpUpgradeHandler.class, TCKReadListener.class, TestServlet.class, TestFilter.class);
5555
}
5656

5757

@@ -74,12 +74,13 @@ public static WebArchive getTestArchive() throws Exception {
7474
*/
7575
@Test
7676
public void upgradeTest() throws Exception {
77-
boolean receivedFirstMessage = false;
78-
boolean receivedSecondMessage = false;
79-
boolean receivedThirdMessage = false;
80-
String EXPECTED_RESPONSE1 = "TCKHttpUpgradeHandler.init";
81-
String EXPECTED_RESPONSE2 = "onDataAvailable|Hello";
82-
String EXPECTED_RESPONSE3 = "onDataAvailable|World";
77+
boolean pass = false;
78+
/*
79+
* This isn't the whole response. Rather it is key, selected parts of the response in the order they are expected to
80+
* appear
81+
*/
82+
String EXPECTED_RESPONSE = "HTTP/1.1 101|x-tracking:|filter-before,servlet-upgrade,filter-after|" +
83+
"TCKHttpUpgradeHandler.init|onDataAvailable|Hello|World";
8384

8485
String requestUrl = getContextRoot() + "/" + getServletName() + " HTTP/1.1";
8586

@@ -88,6 +89,8 @@ public void upgradeTest() throws Exception {
8889
OutputStream output = s.getOutputStream();
8990
InputStream input = s.getInputStream()) {
9091

92+
// Prevent the test waiting for ever if an incorrect response is provided.
93+
s.setSoTimeout(30*1000);
9194

9295
StringBuilder reqStr = new StringBuilder("POST "
9396
+ url.toExternalForm().replace("http://", "").replace(_hostname, "")
@@ -115,39 +118,25 @@ public void upgradeTest() throws Exception {
115118
logger.debug("Consuming the response from the server");
116119

117120
// Consume the response from the server
118-
119121
int len = -1;
120122
byte[] b = new byte[1024];
121123
StringBuilder sb = new StringBuilder();
122-
while ((len = input.read(b)) != -1) {
124+
while (!pass && (len = input.read(b)) != -1) {
123125
String line = new String(b, 0, len);
124126
sb.append(line);
125127
logger.debug("==============Read from server: {} {} {}", CRLF, sb, CRLF);
126-
if (ServletTestUtil.compareString(EXPECTED_RESPONSE1, sb.toString())) {
127-
logger.debug("==============Received first expected response!");
128-
receivedFirstMessage = true;
129-
}
130-
if (ServletTestUtil.compareString(EXPECTED_RESPONSE2, sb.toString())) {
131-
logger.debug("==============Received second expected response!");
132-
receivedSecondMessage = true;
133-
}
134-
if (ServletTestUtil.compareString(EXPECTED_RESPONSE3, sb.toString())) {
135-
logger.debug("==============Received third expected response!");
136-
receivedThirdMessage = true;
137-
}
138-
logger.debug("receivedFirstMessage : {}", Boolean.toString(receivedFirstMessage));
139-
logger.debug("receivedSecondMessage : {}", Boolean.toString(receivedSecondMessage));
140-
logger.debug("receivedThirdMessage : {}", Boolean.toString(receivedThirdMessage));
141-
if (receivedFirstMessage && receivedSecondMessage && receivedThirdMessage) {
142-
break;
143-
}
128+
129+
// Check for the expected messages in the expected order. Exit the loop once they have been observed.
130+
pass = ServletTestUtil.compareString(EXPECTED_RESPONSE, sb.toString());
144131
}
132+
133+
System.out.println(sb.toString());
145134
} catch (Exception ex2) {
146135
logger.error("exception caught: " + ex2.getMessage(), ex2);
147136
}
148137

149138

150-
if (!receivedFirstMessage || !receivedSecondMessage || !receivedThirdMessage) {
139+
if (!pass) {
151140
throw new Exception("Test Failed. ");
152141
}
153142
}

‎tck/tck-runtime/src/main/java/servlet/tck/api/jakarta_servlet_http/httpupgradehandler/TCKHttpUpgradeHandler.java‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -52,27 +52,7 @@ public void destroy() {
5252
}
5353

5454
public void setDelimiter(String delimiter) {
55-
System.out.print("=============== setDelimiter");
55+
System.out.println("===============setDelimiter");
5656
this.delimiter = delimiter;
5757
}
58-
59-
public String getDelimiter() {
60-
System.out.print("=============== getDelimiter");
61-
62-
if (delimiter == null) {
63-
throw new IllegalStateException(
64-
"Delimiter was never initialized - setDelimiter() was not called"
65-
);
66-
}
67-
68-
if (readListener != null) {
69-
String listenerDelimiter = readListener.getDelimiter();
70-
if (!delimiter.equals(listenerDelimiter)) {
71-
throw new IllegalStateException(
72-
"Delimiter mismatch between handler and listener"
73-
);
74-
}
75-
}
76-
return delimiter;
77-
}
7858
}

‎tck/tck-runtime/src/main/java/servlet/tck/api/jakarta_servlet_http/httpupgradehandler/TestFilter.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright (c) 2026 Contributors to the Eclipse Foundation.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0, which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* This Source Code may also be made available under the following Secondary
9+
* Licenses when the conditions for such availability set forth in the
10+
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
11+
* version 2 with the GNU Classpath Exception, which is available at
12+
* https://www.gnu.org/software/classpath/license.html.
13+
*
14+
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
15+
*/
16+
package servlet.tck.api.jakarta_servlet_http.httpupgradehandler;
17+
18+
import java.io.IOException;
19+
20+
import jakarta.servlet.FilterChain;
21+
import jakarta.servlet.GenericFilter;
22+
import jakarta.servlet.ServletException;
23+
import jakarta.servlet.ServletRequest;
24+
import jakarta.servlet.ServletResponse;
25+
import jakarta.servlet.annotation.WebFilter;
26+
import jakarta.servlet.http.HttpServletResponse;
27+
28+
@WebFilter(urlPatterns = {"/TestServlet"})
29+
public class TestFilter extends GenericFilter {
30+
31+
private static final long serialVersionUID = 1L;
32+
33+
public static final String TRACKING_HEADER_NAME = "x-tracking";
34+
35+
@Override
36+
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
37+
throws IOException, ServletException {
38+
39+
/*
40+
* The case - and the test - will fail for non-HTTP requests. This is OK since this is an HTTP upgrade test.
41+
*/
42+
HttpServletResponse hResponse = (HttpServletResponse) response;
43+
44+
String value = hResponse.getHeader(TRACKING_HEADER_NAME);
45+
// value should be null here but just in case...
46+
if (value == null) {
47+
value = "filter-before";
48+
} else {
49+
value = value + ",filter-before";
50+
}
51+
hResponse.setHeader(TRACKING_HEADER_NAME, value);
52+
53+
chain.doFilter(request, response);
54+
55+
value = hResponse.getHeader(TRACKING_HEADER_NAME);
56+
// value should not be null here but just in case...
57+
if (value == null) {
58+
value = "filter-after";
59+
} else {
60+
value = value + ",filter-after";
61+
}
62+
hResponse.setHeader(TRACKING_HEADER_NAME, value);
63+
}
64+
}

‎tck/tck-runtime/src/main/java/servlet/tck/api/jakarta_servlet_http/httpupgradehandler/TestServlet.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,18 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
3737
response.setStatus(101);
3838
response.setHeader("Upgrade", "YES");
3939
response.setHeader("Connection", "Upgrade");
40-
TCKHttpUpgradeHandler handler = request
41-
.upgrade(TCKHttpUpgradeHandler.class);
40+
TCKHttpUpgradeHandler handler = request.upgrade(TCKHttpUpgradeHandler.class);
4241
handler.setDelimiter("/");
42+
43+
String value = response.getHeader(TestFilter.TRACKING_HEADER_NAME);
44+
// value should not be null here but just in case...
45+
if (value == null) {
46+
value = "servlet-upgrade";
47+
} else {
48+
value = value + ",servlet-upgrade";
49+
}
50+
response.setHeader(TestFilter.TRACKING_HEADER_NAME, value);
51+
4352
} else {
4453
response.getWriter().println("No upgrade");
4554
response.getWriter().println("End of Test");

0 commit comments

Comments
 (0)