Use wrapped HttpServlet req/resp instead of base ones from ServletContextRequest

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2022-08-04 10:10:59 +10:00
parent 6c319f339b
commit a391dc4aa9
8 changed files with 188 additions and 48 deletions

View File

@ -31,6 +31,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicReference;
import jakarta.servlet.AsyncContext;
import jakarta.servlet.AsyncListener;
@ -74,6 +75,7 @@ import org.eclipse.jetty.server.handler.ContextRequest;
import org.eclipse.jetty.server.handler.ContextResponse;
import org.eclipse.jetty.session.Session;
import org.eclipse.jetty.session.SessionManager;
import org.eclipse.jetty.util.Attachable;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.Fields;
import org.eclipse.jetty.util.HostPort;
@ -82,7 +84,7 @@ import org.eclipse.jetty.util.URIUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class ServletContextRequest extends ContextRequest implements Runnable
public class ServletContextRequest extends ContextRequest implements Runnable, Attachable
{
public static final String __MULTIPART_CONFIG_ELEMENT = "org.eclipse.jetty.multipartConfig";
@ -122,6 +124,7 @@ public class ServletContextRequest extends ContextRequest implements Runnable
final HttpInput _httpInput;
final String _pathInContext;
Charset _queryEncoding;
final AtomicReference<Object> _attachment = new AtomicReference<>();
final List<ServletRequestAttributeListener> _requestAttributeListeners = new ArrayList<>();
@ -140,6 +143,18 @@ public class ServletContextRequest extends ContextRequest implements Runnable
_pathInContext = pathInContext;
}
@Override
public Object getAttachment()
{
return _attachment.get();
}
@Override
public void setAttachment(Object attachment)
{
_attachment.set(attachment);
}
@Override
public void process(Request request, Response response, Callback callback) throws Exception
{

View File

@ -20,6 +20,7 @@ import java.util.Collection;
import java.util.EnumSet;
import java.util.Iterator;
import java.util.Locale;
import java.util.concurrent.atomic.AtomicReference;
import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletOutputStream;
@ -53,6 +54,7 @@ import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.handler.ContextResponse;
import org.eclipse.jetty.session.Session;
import org.eclipse.jetty.session.SessionManager;
import org.eclipse.jetty.util.Attachable;
import org.eclipse.jetty.util.Blocker;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.FutureCallback;
@ -60,7 +62,7 @@ import org.eclipse.jetty.util.SharedBlockingCallback;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
public class ServletContextResponse extends ContextResponse
public class ServletContextResponse extends ContextResponse implements Attachable
{
private static final int __MIN_BUFFER_SIZE = 1;
private static final HttpField __EXPIRES_01JAN1970 = new PreEncodedHttpField(HttpHeader.EXPIRES, DateGenerator.__01Jan1970);
@ -85,7 +87,8 @@ public class ServletContextResponse extends ContextResponse
private ResponseWriter _writer;
private long _contentLength = -1;
final AtomicReference<Object> _attachment = new AtomicReference<>();
public static ServletContextResponse getBaseResponse(ServletResponse response)
{
if (response instanceof ServletApiResponse)
@ -112,6 +115,18 @@ public class ServletContextResponse extends ContextResponse
_httpServletResponse = new ServletApiResponse(response);
}
@Override
public Object getAttachment()
{
return _attachment.get();
}
@Override
public void setAttachment(Object attachment)
{
_attachment.set(attachment);
}
public HttpOutput getHttpOutput()
{
return _httpOutput;

View File

@ -24,6 +24,7 @@ import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
import org.eclipse.jetty.ee10.servlet.ServletContextRequest;
import org.eclipse.jetty.ee10.servlet.ServletContextResponse;
import org.eclipse.jetty.ee10.websocket.server.internal.DelegatedServerUpgradeRequest;
import org.eclipse.jetty.ee10.websocket.server.internal.DelegatedServerUpgradeResponse;
import org.eclipse.jetty.ee10.websocket.server.internal.JettyServerFrameHandlerFactory;
@ -182,23 +183,32 @@ public abstract class JettyWebSocketServlet extends HttpServlet
ServletContextRequest request = ServletContextRequest.getBaseRequest(req);
if (request == null)
throw new IllegalStateException("Base Request not available");
ServletContextResponse response = request.getResponse();
// provide a null default customizer the customizer will be on the negotiator in the mapping
try (Blocker.Callback callback = Blocker.callback())
// Do preliminary check before proceeding to attempt an upgrade.
if (mapping.getHandshaker().isWebSocketUpgradeRequest(request))
{
if (mapping.upgrade(request, request.getResponse(), callback, null))
// provide a null default customizer the customizer will be on the negotiator in the mapping
try (Blocker.Callback callback = Blocker.callback())
{
callback.block();
return;
// Set the wrapped req and resp as attachments on the ServletContext Request/Response, so they
// are accessible when websocket-core calls back the Jetty WebSocket creator.
request.setAttachment(req);
response.setAttachment(resp);
if (mapping.upgrade(request, response, callback, null))
{
callback.block();
return;
}
}
finally
{
request.setAttachment(null);
response.setAttachment(null);
}
}
// If we reach this point, it means we had an incoming request to upgrade
// but it was either not a proper websocket upgrade, or it was possibly rejected
// due to incoming request constraints (controlled by WebSocketCreator)
if (resp.isCommitted())
return;
// Handle normally
super.service(req, resp);
}

View File

@ -52,13 +52,13 @@ public class DelegatedServerUpgradeRequest implements JettyServerUpgradeRequest
public DelegatedServerUpgradeRequest(ServerUpgradeRequest request)
{
this(request, Request.as(request, ServletContextRequest.class).getHttpServletRequest());
}
ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class);
Object attachment = servletContextRequest.getAttachment();
if (!(attachment instanceof HttpServletRequest))
throw new IllegalArgumentException("correct attachment not set on ServletContextRequest");
public DelegatedServerUpgradeRequest(ServerUpgradeRequest request, HttpServletRequest servletRequest)
{
this.upgradeRequest = request;
this.httpServletRequest = servletRequest;
this.httpServletRequest = (HttpServletRequest)attachment;
this.queryString = httpServletRequest.getQueryString();
try
@ -145,13 +145,13 @@ public class DelegatedServerUpgradeRequest implements JettyServerUpgradeRequest
@Override
public String getMethod()
{
return upgradeRequest.getMethod();
return httpServletRequest.getMethod();
}
@Override
public String getOrigin()
{
return upgradeRequest.getHeaders().get(HttpHeader.ORIGIN);
return httpServletRequest.getHeader(HttpHeader.ORIGIN.asString());
}
@Override

View File

@ -21,26 +21,35 @@ import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.ee10.servlet.ServletContextResponse;
import org.eclipse.jetty.ee10.websocket.api.ExtensionConfig;
import org.eclipse.jetty.ee10.websocket.common.JettyExtensionConfig;
import org.eclipse.jetty.ee10.websocket.server.JettyServerUpgradeResponse;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.util.Blocker;
import org.eclipse.jetty.websocket.core.server.ServerUpgradeResponse;
public class DelegatedServerUpgradeResponse implements JettyServerUpgradeResponse
{
private final ServerUpgradeResponse upgradeResponse;
private final HttpServletResponse httpServletResponse;
public DelegatedServerUpgradeResponse(ServerUpgradeResponse response)
{
upgradeResponse = response;
ServletContextResponse servletContextResponse = Response.as(response, ServletContextResponse.class);
Object attachment = servletContextResponse.getAttachment();
if (!(attachment instanceof HttpServletResponse))
throw new IllegalArgumentException("correct attachment not set on ServletContextResponse");
this.httpServletResponse = (HttpServletResponse)attachment;
}
@Override
public void addHeader(String name, String value)
{
// TODO: This should go to the httpServletResponse for headers but then it won't do interception of the websocket headers
// which are done through the jetty-core Response wrapping ServerUpgradeResponse done by websocket-core.
upgradeResponse.getHeaders().add(name, value);
}
@ -97,17 +106,13 @@ public class DelegatedServerUpgradeResponse implements JettyServerUpgradeRespons
@Override
public int getStatusCode()
{
return upgradeResponse.getStatus();
return httpServletResponse.getStatus();
}
@Override
public void sendForbidden(String message) throws IOException
{
try (Blocker.Callback callback = Blocker.callback())
{
Response.writeError(upgradeResponse.getRequest(), upgradeResponse, callback, HttpStatus.FORBIDDEN_403, message);
callback.block();
}
httpServletResponse.sendError(HttpStatus.FORBIDDEN_403, message);
}
@Override
@ -127,22 +132,18 @@ public class DelegatedServerUpgradeResponse implements JettyServerUpgradeRespons
@Override
public void setStatusCode(int statusCode)
{
upgradeResponse.setStatus(statusCode);
httpServletResponse.setStatus(statusCode);
}
@Override
public boolean isCommitted()
{
return upgradeResponse.isCommitted();
return httpServletResponse.isCommitted();
}
@Override
public void sendError(int statusCode, String message) throws IOException
{
try (Blocker.Callback callback = Blocker.callback())
{
Response.writeError(upgradeResponse.getRequest(), upgradeResponse, callback, statusCode, message);
callback.block();
}
httpServletResponse.sendError(statusCode, message);
}
}

View File

@ -14,12 +14,9 @@
package org.eclipse.jetty.ee10.websocket.server.internal;
import jakarta.servlet.ServletContext;
import jakarta.servlet.http.HttpServletRequest;
import org.eclipse.jetty.ee10.servlet.ServletContextRequest;
import org.eclipse.jetty.ee10.websocket.common.JettyWebSocketFrameHandler;
import org.eclipse.jetty.ee10.websocket.common.JettyWebSocketFrameHandlerFactory;
import org.eclipse.jetty.ee10.websocket.server.JettyWebSocketServerContainer;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.websocket.core.FrameHandler;
import org.eclipse.jetty.websocket.core.WebSocketComponents;
import org.eclipse.jetty.websocket.core.server.FrameHandlerFactory;
@ -42,11 +39,8 @@ public class JettyServerFrameHandlerFactory extends JettyWebSocketFrameHandlerFa
@Override
public FrameHandler newFrameHandler(Object websocketPojo, ServerUpgradeRequest upgradeRequest, ServerUpgradeResponse upgradeResponse)
{
ServletContextRequest servletContextRequest = Request.as(upgradeRequest, ServletContextRequest.class);
HttpServletRequest httpServletRequest = servletContextRequest.getHttpServletRequest();
JettyWebSocketFrameHandler frameHandler = super.newJettyFrameHandler(websocketPojo);
frameHandler.setUpgradeRequest(new DelegatedServerUpgradeRequest(upgradeRequest, httpServletRequest));
frameHandler.setUpgradeRequest(new DelegatedServerUpgradeRequest(upgradeRequest));
frameHandler.setUpgradeResponse(new DelegatedServerUpgradeResponse(upgradeResponse));
return frameHandler;
}

View File

@ -0,0 +1,90 @@
//
// ========================================================================
// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//
package org.eclipse.jetty.ee10.websocket.tests;
import java.net.URI;
import java.time.Duration;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
import org.eclipse.jetty.ee10.websocket.api.Session;
import org.eclipse.jetty.ee10.websocket.api.StatusCode;
import org.eclipse.jetty.ee10.websocket.client.WebSocketClient;
import org.eclipse.jetty.ee10.websocket.server.config.JettyWebSocketServletContainerInitializer;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class SimpleEchoTest
{
private Server _server;
private WebSocketClient _client;
private ServerConnector _connector;
@BeforeEach
public void start() throws Exception
{
_server = new Server();
_connector = new ServerConnector(_server);
_server.addConnector(_connector);
ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
contextHandler.setContextPath("/");
JettyWebSocketServletContainerInitializer.configure(contextHandler, ((servletContext, container) ->
{
container.setIdleTimeout(Duration.ZERO);
container.addMapping("/", EchoSocket.class);
}));
_server.setHandler(contextHandler);
_server.start();
_client = new WebSocketClient();
_client.start();
}
@AfterEach
public void stop() throws Exception
{
_client.stop();
_server.stop();
}
@Test
public void testEcho() throws Exception
{
int timeout = 3;
_client.setIdleTimeout(Duration.ofSeconds(timeout));
_client.setConnectTimeout(Duration.ofSeconds(timeout).toMillis());
URI uri = new URI("ws://localhost:" + _connector.getLocalPort());
EventSocket clientEndpoint = new EventSocket();
Session session = _client.connect(clientEndpoint, uri).get(timeout, TimeUnit.SECONDS);
session.setIdleTimeout(Duration.ofSeconds(timeout));
String message = "hello world 1234";
session.getRemote().sendString(message);
String received = clientEndpoint.textMessages.poll(timeout, TimeUnit.SECONDS);
assertThat(received, equalTo(message));
session.close();
assertTrue(clientEndpoint.closeLatch.await(timeout, TimeUnit.SECONDS));
assertThat(clientEndpoint.closeCode, equalTo(StatusCode.NORMAL));
}
}

View File

@ -30,6 +30,7 @@ import org.eclipse.jetty.ee10.servlet.FilterHolder;
import org.eclipse.jetty.ee10.servlet.FilterMapping;
import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
import org.eclipse.jetty.ee10.servlet.ServletContextRequest;
import org.eclipse.jetty.ee10.servlet.ServletContextResponse;
import org.eclipse.jetty.ee10.servlet.ServletHandler;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.util.Blocker;
@ -158,16 +159,30 @@ public class WebSocketUpgradeFilter implements Filter, Dumpable
ServletContextRequest baseRequest = ServletContextRequest.getBaseRequest(request);
if (baseRequest == null)
throw new IllegalStateException("Base Request not available");
ServletContextResponse baseResponse = baseRequest.getResponse();
// provide a null default customizer the customizer will be on the negotiator in the mapping
try (Blocker.Callback callback = Blocker.callback())
// Do preliminary check before proceeding to attempt an upgrade.
if (mappings.getHandshaker().isWebSocketUpgradeRequest(baseRequest))
{
if (mappings.upgrade(baseRequest, baseRequest.getResponse(), callback, defaultCustomizer))
// provide a null default customizer the customizer will be on the negotiator in the mapping
try (Blocker.Callback callback = Blocker.callback())
{
callback.block();
return;
// Set the wrapped req and resp as attachments on the ServletContext Request/Response, so they
// are accessible when websocket-core calls back the Jetty WebSocket creator.
baseRequest.setAttachment(request);
baseResponse.setAttachment(response);
if (mappings.upgrade(baseRequest, baseResponse, callback, null))
{
callback.block();
return;
}
}
finally
{
baseRequest.setAttachment(null);
baseResponse.setAttachment(null);
}
callback.succeeded(); // TODO this is wasteful making a blocker on every request, even if it is not used. At leasts should be shared... but better to detect if we might need to upgrade first?
}
// If we reach this point, it means we had an incoming request to upgrade