Skip to content

Commit 28bd2cd

Browse files
lachlan-robertslachlan-roberts
committed
Merge PR jetty#12441 to EE11
Signed-off-by: Lachlan Roberts <[email protected]>
1 parent c5990fb commit 28bd2cd

File tree

12 files changed

+286
-69
lines changed

12 files changed

+286
-69
lines changed

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/client/internal/JsrUpgradeListener.java

+4-30
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@
1313

1414
package org.eclipse.jetty.ee11.websocket.jakarta.client.internal;
1515

16-
import java.util.ArrayList;
17-
import java.util.Collections;
18-
import java.util.HashMap;
1916
import java.util.List;
2017
import java.util.Map;
2118

@@ -41,23 +38,11 @@ public void onHandshakeRequest(Request request)
4138
if (configurator == null)
4239
return;
4340

44-
HttpFields fields = request.getHeaders();
45-
Map<String, List<String>> originalHeaders = new HashMap<>();
46-
fields.forEach(field ->
47-
{
48-
originalHeaders.putIfAbsent(field.getName(), new ArrayList<>());
49-
List<String> values = originalHeaders.get(field.getName());
50-
Collections.addAll(values, field.getValues());
51-
});
52-
53-
// Give headers to configurator
54-
configurator.beforeRequest(originalHeaders);
55-
56-
// Reset headers on HttpRequest per configurator
5741
request.headers(headers ->
5842
{
59-
headers.clear();
60-
originalHeaders.forEach(headers::put);
43+
// Give headers to configurator
44+
Map<String, List<String>> headersMap = HttpFields.asMap(headers);
45+
configurator.beforeRequest(headersMap);
6146
});
6247
}
6348

@@ -67,18 +52,7 @@ public void onHandshakeResponse(Request request, Response response)
6752
if (configurator == null)
6853
return;
6954

70-
HandshakeResponse handshakeResponse = () ->
71-
{
72-
Map<String, List<String>> ret = new HashMap<>();
73-
response.getHeaders().forEach(field ->
74-
{
75-
ret.putIfAbsent(field.getName(), new ArrayList<>());
76-
List<String> values = ret.get(field.getName());
77-
Collections.addAll(values, field.getValues());
78-
});
79-
return ret;
80-
};
81-
55+
HandshakeResponse handshakeResponse = () -> HttpFields.asMap(response.getHeaders());
8256
configurator.afterResponse(handshakeResponse);
8357
}
8458
}

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/server/JakartaWebSocketServerContainer.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -322,9 +322,9 @@ public void upgradeHttpToWebSocket(Object httpServletRequest, Object httpServlet
322322
servletContextRequest.setAttribute(WebSocketConstants.WEBSOCKET_WRAPPED_RESPONSE_ATTRIBUTE, response);
323323

324324
if (handshaker.upgradeRequest(negotiator, servletContextRequest, servletContextResponse, callback, components, defaultCustomizer))
325-
{
326325
callback.block();
327-
}
326+
else
327+
throw new IllegalStateException("Invalid WebSocket Upgrade Request");
328328
}
329329
finally
330330
{

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/server/internal/JakartaWebSocketCreator.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,6 @@ public Map<String, Object> getUserProperties()
156156

157157
// [JSR] Step 5: Call modifyHandshake
158158
configurator.modifyHandshake(config, jsrHandshakeRequest, jsrHandshakeResponse);
159-
// Set modified headers Map back into response properly
160-
jsrHandshakeResponse.setHeaders(jsrHandshakeResponse.getHeaders());
161159

162160
try
163161
{
@@ -168,7 +166,8 @@ public Map<String, Object> getUserProperties()
168166
}
169167
catch (Throwable x)
170168
{
171-
LOG.warn("Unable to create websocket: {}", config.getEndpointClass().getName(), x);
169+
if (LOG.isDebugEnabled())
170+
LOG.debug("Unable to create websocket: {}", config.getEndpointClass().getName(), x);
172171
callback.failed(x);
173172
return null;
174173
}

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/server/internal/JsrHandshakeRequest.java

+2-6
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,14 @@
1515

1616
import java.net.URI;
1717
import java.security.Principal;
18-
import java.util.ArrayList;
19-
import java.util.Collections;
2018
import java.util.HashMap;
2119
import java.util.List;
2220
import java.util.Map;
23-
import java.util.stream.Collectors;
2421

2522
import jakarta.servlet.http.HttpServletRequest;
2623
import jakarta.websocket.server.HandshakeRequest;
2724
import org.eclipse.jetty.ee11.websocket.jakarta.server.JakartaWebSocketServerContainer;
25+
import org.eclipse.jetty.http.HttpFields;
2826
import org.eclipse.jetty.http.pathmap.PathSpec;
2927
import org.eclipse.jetty.server.Request;
3028
import org.eclipse.jetty.util.Fields;
@@ -47,9 +45,7 @@ public JsrHandshakeRequest(ServerUpgradeRequest req)
4745
@Override
4846
public Map<String, List<String>> getHeaders()
4947
{
50-
Map<String, List<String>> headers = delegate.getHeaders().getFieldNamesCollection().stream()
51-
.collect(Collectors.toMap((name) -> name, (name) -> new ArrayList<>(delegate.getHeaders().getValuesList(name))));
52-
return Collections.unmodifiableMap(headers);
48+
return HttpFields.asMap(delegate.getHeaders());
5349
}
5450

5551
@Override

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/server/internal/JsrHandshakeResponse.java

+2-9
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,11 @@
1313

1414
package org.eclipse.jetty.ee11.websocket.jakarta.server.internal;
1515

16-
import java.util.ArrayList;
1716
import java.util.List;
1817
import java.util.Map;
19-
import java.util.stream.Collectors;
2018

2119
import jakarta.websocket.HandshakeResponse;
20+
import org.eclipse.jetty.http.HttpFields;
2221
import org.eclipse.jetty.websocket.core.server.ServerUpgradeResponse;
2322

2423
public class JsrHandshakeResponse implements HandshakeResponse
@@ -29,18 +28,12 @@ public class JsrHandshakeResponse implements HandshakeResponse
2928
public JsrHandshakeResponse(ServerUpgradeResponse resp)
3029
{
3130
this.delegate = resp;
32-
this.headers = delegate.getHeaders().getFieldNamesCollection().stream()
33-
.collect(Collectors.toMap((name) -> name, (name) -> new ArrayList<>(delegate.getHeaders().getValuesList(name))));
31+
this.headers = HttpFields.asMap(resp.getHeaders());
3432
}
3533

3634
@Override
3735
public Map<String, List<String>> getHeaders()
3836
{
3937
return headers;
4038
}
41-
42-
public void setHeaders(Map<String, List<String>> headers)
43-
{
44-
headers.forEach((key, values) -> delegate.getHeaders().put(key, values));
45-
}
4639
}

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/tests/UpgradeHeadersTest.java

+138
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
//
2+
// ========================================================================
3+
// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
4+
//
5+
// This program and the accompanying materials are made available under the
6+
// terms of the Eclipse Public License v. 2.0 which is available at
7+
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
8+
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
9+
//
10+
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
11+
// ========================================================================
12+
//
13+
14+
package org.eclipse.jetty.ee11.websocket.jakarta.tests;
15+
16+
import java.net.URI;
17+
import java.util.List;
18+
import java.util.Map;
19+
import java.util.concurrent.TimeUnit;
20+
21+
import jakarta.websocket.ClientEndpointConfig;
22+
import jakarta.websocket.Endpoint;
23+
import jakarta.websocket.EndpointConfig;
24+
import jakarta.websocket.HandshakeResponse;
25+
import jakarta.websocket.Session;
26+
import jakarta.websocket.server.HandshakeRequest;
27+
import jakarta.websocket.server.ServerEndpointConfig;
28+
import org.eclipse.jetty.ee11.servlet.ServletContextHandler;
29+
import org.eclipse.jetty.ee11.websocket.jakarta.client.JakartaWebSocketClientContainer;
30+
import org.eclipse.jetty.ee11.websocket.jakarta.server.config.JakartaWebSocketServletContainerInitializer;
31+
import org.eclipse.jetty.server.Server;
32+
import org.eclipse.jetty.server.ServerConnector;
33+
import org.junit.jupiter.api.AfterEach;
34+
import org.junit.jupiter.api.Test;
35+
36+
import static org.junit.jupiter.api.Assertions.assertTrue;
37+
38+
public class UpgradeHeadersTest
39+
{
40+
private Server _server;
41+
private JakartaWebSocketClientContainer _client;
42+
private ServerConnector _connector;
43+
44+
public static class MyEndpoint extends Endpoint
45+
{
46+
@Override
47+
public void onOpen(Session session, EndpointConfig config)
48+
{
49+
}
50+
}
51+
52+
public void start(ServerEndpointConfig.Configurator configurator) throws Exception
53+
{
54+
_server = new Server();
55+
_connector = new ServerConnector(_server);
56+
_server.addConnector(_connector);
57+
58+
ServletContextHandler contextHandler = new ServletContextHandler();
59+
_server.setHandler(contextHandler);
60+
JakartaWebSocketServletContainerInitializer.configure(contextHandler, (context, container) ->
61+
{
62+
container.addEndpoint(ServerEndpointConfig.Builder
63+
.create(MyEndpoint.class, "/")
64+
.configurator(configurator)
65+
.build());
66+
});
67+
68+
_server.start();
69+
_client = new JakartaWebSocketClientContainer();
70+
_client.start();
71+
}
72+
73+
@AfterEach
74+
public void after() throws Exception
75+
{
76+
_client.stop();
77+
_server.stop();
78+
}
79+
80+
@Test
81+
public void testCaseInsensitiveUpgradeHeaders() throws Exception
82+
{
83+
ClientEndpointConfig.Configurator configurator = new ClientEndpointConfig.Configurator()
84+
{
85+
@Override
86+
public void beforeRequest(Map<String, List<String>> headers)
87+
{
88+
// Verify that existing headers can be accessed in a case-insensitive way.
89+
if (headers.get("cOnnEcTiOn") == null)
90+
throw new IllegalStateException("No Connection Header on client Request");
91+
headers.put("sentHeader", List.of("value123"));
92+
}
93+
94+
@Override
95+
public void afterResponse(HandshakeResponse hr)
96+
{
97+
if (hr.getHeaders().get("MyHeAdEr") == null)
98+
throw new IllegalStateException("No custom Header on HandshakeResponse");
99+
if (hr.getHeaders().get("cOnnEcTiOn") == null)
100+
throw new IllegalStateException("No Connection Header on HandshakeRequest");
101+
}
102+
};
103+
104+
start(new ServerEndpointConfig.Configurator()
105+
{
106+
@Override
107+
public void modifyHandshake(ServerEndpointConfig sec, HandshakeRequest request, HandshakeResponse response)
108+
{
109+
// Verify that existing headers can be accessed in a case-insensitive way.
110+
if (request.getHeaders().get("cOnnEcTiOn") == null)
111+
throw new IllegalStateException("No Connection Header on HandshakeRequest");
112+
if (response.getHeaders().get("sErVeR") == null)
113+
throw new IllegalStateException("No Server Header on HandshakeResponse");
114+
115+
// Verify custom header sent from client.
116+
if (request.getHeaders().get("SeNtHeadEr") == null)
117+
throw new IllegalStateException("No sent Header on HandshakeResponse");
118+
119+
// Add custom response header.
120+
response.getHeaders().put("myHeader", List.of("foobar"));
121+
if (response.getHeaders().get("MyHeAdEr") == null)
122+
throw new IllegalStateException("No custom Header on HandshakeResponse");
123+
124+
super.modifyHandshake(sec, request, response);
125+
}
126+
});
127+
128+
WSEndpointTracker clientEndpoint = new WSEndpointTracker(){};
129+
ClientEndpointConfig clientConfig = ClientEndpointConfig.Builder.create().configurator(configurator).build();
130+
URI uri = URI.create("ws://localhost:" + _connector.getLocalPort());
131+
132+
// If any of the above throw it would fail to upgrade to websocket.
133+
Session session = _client.connectToServer(clientEndpoint, clientConfig, uri);
134+
assertTrue(clientEndpoint.openLatch.await(5, TimeUnit.SECONDS));
135+
session.close();
136+
assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS));
137+
}
138+
}

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/tests/client/AnnotatedClientEndpointTest.java

-8
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,12 @@
1515

1616
import java.io.IOException;
1717
import java.nio.ByteBuffer;
18-
import java.util.Collections;
1918
import java.util.Date;
2019

2120
import jakarta.websocket.ClientEndpoint;
2221
import jakarta.websocket.ClientEndpointConfig;
2322
import jakarta.websocket.ContainerProvider;
2423
import jakarta.websocket.EndpointConfig;
25-
import jakarta.websocket.HandshakeResponse;
2624
import jakarta.websocket.OnMessage;
2725
import jakarta.websocket.OnOpen;
2826
import jakarta.websocket.Session;
@@ -75,12 +73,6 @@ public Date onBinary(ByteBuffer buf)
7573

7674
public static class AnnotatedEndpointConfigurator extends ClientEndpointConfig.Configurator
7775
{
78-
@Override
79-
public void afterResponse(HandshakeResponse hr)
80-
{
81-
hr.getHeaders().put("X-Test", Collections.singletonList("Extra"));
82-
super.afterResponse(hr);
83-
}
8476
}
8577

8678
private static CoreServer server;

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee11/websocket/server/JettyWebSocketServerContainer.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ public void addMapping(String pathSpec, JettyWebSocketCreator creator)
158158
}
159159
catch (Throwable t)
160160
{
161+
if (LOG.isDebugEnabled())
162+
LOG.debug("Could not create WebSocket endpoint", t);
161163
cb.failed(t);
162164
return null;
163165
}
@@ -203,11 +205,14 @@ public boolean upgrade(JettyWebSocketCreator creator, HttpServletRequest request
203205
try
204206
{
205207
Object webSocket = creator.createWebSocket(new DelegatedServerUpgradeRequest(req), new DelegatedServerUpgradeResponse(resp));
206-
cb.succeeded();
208+
if (webSocket == null)
209+
cb.succeeded();
207210
return webSocket;
208211
}
209212
catch (Throwable t)
210213
{
214+
if (LOG.isDebugEnabled())
215+
LOG.debug("Could not create WebSocket endpoint", t);
211216
cb.failed(t);
212217
return null;
213218
}

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee11/websocket/server/JettyWebSocketServlet.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,8 @@ public Object createWebSocket(ServerUpgradeRequest upgradeRequest, ServerUpgrade
303303
try
304304
{
305305
Object webSocket = creator.createWebSocket(request, response);
306-
callback.succeeded();
306+
if (webSocket == null)
307+
callback.succeeded();
307308
return webSocket;
308309
}
309310
catch (Throwable t)

‎jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee11/websocket/server/internal/DelegatedServerUpgradeRequest.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import java.net.URI;
1919
import java.security.Principal;
2020
import java.security.cert.X509Certificate;
21-
import java.util.ArrayList;
2221
import java.util.Arrays;
2322
import java.util.Collections;
2423
import java.util.Enumeration;
@@ -33,6 +32,7 @@
3332
import jakarta.servlet.http.HttpSession;
3433
import org.eclipse.jetty.ee11.websocket.server.JettyServerUpgradeRequest;
3534
import org.eclipse.jetty.http.BadMessageException;
35+
import org.eclipse.jetty.http.HttpFields;
3636
import org.eclipse.jetty.http.HttpHeader;
3737
import org.eclipse.jetty.util.URIUtil;
3838
import org.eclipse.jetty.websocket.api.ExtensionConfig;
@@ -121,9 +121,7 @@ public int getHeaderInt(String name)
121121
@Override
122122
public Map<String, List<String>> getHeaders()
123123
{
124-
Map<String, List<String>> headers = upgradeRequest.getHeaders().getFieldNamesCollection().stream()
125-
.collect(Collectors.toMap((name) -> name, (name) -> new ArrayList<>(getHeaders(name))));
126-
return Collections.unmodifiableMap(headers);
124+
return HttpFields.asMap(upgradeRequest.getHeaders());
127125
}
128126

129127
@Override

0 commit comments

Comments
 (0)