Skip to content
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

implement servlet upgrade for ee10 #10128

Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
66cc353
Various Cleanup in ServletChannel
gregw Jul 5, 2023
adb2fb2
Various Cleanup in ServletChannel
gregw Jul 5, 2023
a02dd13
Various Cleanup in ServletChannel
gregw Jul 5, 2023
8faf41e
make ServletChannel error dispatch async
lachlan-roberts Jul 6, 2023
de21dd7
Various Cleanup in ServletChannel
gregw Jul 5, 2023
073e95a
scheduleDispatch after error handler callback
lachlan-roberts Jul 7, 2023
c980ab5
use AtomicBoolean instead of AtomicInteger for error dispatch callback
lachlan-roberts Jul 17, 2023
0e2a0c1
remove checkAndPrepareUpgrade() from ServletChannel
lachlan-roberts Jul 17, 2023
68439bf
implement servlet upgrade for ee10
lachlan-roberts Jul 19, 2023
452a71b
adjust implementation to fix tests
lachlan-roberts Jul 20, 2023
1a178a2
Merge remote-tracking branch 'origin/jetty-12.0.x' into experiment/je…
lachlan-roberts Aug 18, 2023
f222c45
re-enable ServletUpgradeTest for ee10
lachlan-roberts Aug 18, 2023
b5ae22f
Merge remote-tracking branch 'origin/jetty-12.0.x' into experiment/je…
lachlan-roberts Oct 2, 2023
7911d62
Merge remote-tracking branch 'origin/jetty-12.0.x' into experiment/je…
lachlan-roberts Oct 3, 2023
1869ca6
Merge remote-tracking branch 'origin/jetty-12.0.x' into experiment/je…
lachlan-roberts Dec 11, 2023
b6286b2
Merge remote-tracking branch 'origin/jetty-12.0.x' into experiment/je…
lachlan-roberts Jun 14, 2024
c942a7e
wip
lachlan-roberts Jul 17, 2024
36a85a8
add listener to ServletChannel to properly implement servlet upgrade
lachlan-roberts Jul 24, 2024
ef6021d
fix checkstyle issues
lachlan-roberts Jul 24, 2024
ad35ccc
Merge remote-tracking branch 'origin/jetty-12.0.x' into experiment/je…
lachlan-roberts Aug 6, 2024
af300f0
PR #10128 - change ordering in the ServletChannel complete listener
lachlan-roberts Aug 6, 2024
aa8eeaa
changes to ServletUpgradeTest
lachlan-roberts Aug 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1920,8 +1920,8 @@ public void reset()

public void servletUpgrade()
{
setState(State.CONTENT);
_endOfContent = EndOfContent.UNKNOWN_CONTENT;
setState(State.EOF_CONTENT);
_endOfContent = EndOfContent.EOF_CONTENT;
_contentLength = -1;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.HttpConnection;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.Utf8StringBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.RetainableByteBuffer;
import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.ProcessorUtils;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.QuotedCSVParser;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.HostPort;
import org.eclipse.jetty.util.Index;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// ========================================================================
//

package org.eclipse.jetty.server.internal;
package org.eclipse.jetty.server;

import java.io.IOException;
import java.nio.ByteBuffer;
Expand Down Expand Up @@ -55,16 +55,7 @@
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.io.WriteFlusher;
import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.server.AbstractMetaDataConnection;
import org.eclipse.jetty.server.ConnectionFactory;
import org.eclipse.jetty.server.ConnectionMetaData;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpStream;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.TunnelSupport;
import org.eclipse.jetty.server.internal.HttpChannelState;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.HostPort;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.annotation.Name;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnection;
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
import org.eclipse.jetty.server.HttpStream;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.RequestLog;
Expand Down Expand Up @@ -1209,7 +1210,12 @@ public void succeeded()
httpChannel.lockedStreamSendCompleted(true);
}
if (callback != null)
httpChannel._serializedInvoker.run(callback::succeeded);
{
// Do not use SerializedInvoker here because application code running within
// SerializedInvoker could issue blocking write() or close() that would deadlock.
// We must execute in case of recursive write within the callback.
getRequest().getContext().execute(callback::succeeded);
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
Expand Down Expand Up @@ -1237,7 +1243,12 @@ public void failed(Throwable x)
httpChannel.lockedStreamSendCompleted(false);
}
if (callback != null)
httpChannel._serializedInvoker.run(() -> callback.failed(x));
{
// Do not use SerializedInvoker here because application code running within
// SerializedInvoker could issue blocking write() or close() that would deadlock.
// We must execute in case of recursive write within the callback.
getRequest().getContext().execute(() -> callback.failed(x));
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Callback;
import org.junit.jupiter.api.BeforeEach;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.eclipse.jetty.io.ManagedSelector;
import org.eclipse.jetty.io.SocketChannelEndPoint;
import org.eclipse.jetty.server.internal.HttpChannelState;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.NanoTime;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Callback;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.logging.StacklessLogging;
import org.eclipse.jetty.server.handler.DumpHandler;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.NanoTime;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.eclipse.jetty.logging.StacklessLogging;
import org.eclipse.jetty.server.handler.EchoHandler;
import org.eclipse.jetty.server.handler.HelloHandler;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Blocker;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.logging.StacklessLogging;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.component.LifeCycle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.eclipse.jetty.io.QuietException;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.internal.HttpChannelState;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Blocker;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.component.LifeCycle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Callback;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.ContextHandlerCollection;
import org.eclipse.jetty.server.handler.StatisticsHandler;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.FutureCallback;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ public boolean handle(Request request, Response response, Callback callback) thr
ByteArrayOutputStream out = new ByteArrayOutputStream(8192);
new Throwable().printStackTrace(new PrintStream(out));
String stack = out.toString(StandardCharsets.ISO_8859_1);
assertThat(stack, containsString("org.eclipse.jetty.server.internal.HttpConnection.onFillable"));
assertThat(stack, containsString("org.eclipse.jetty.server.HttpConnection.onFillable"));
assertThat(stack, containsString("org.eclipse.jetty.server.handler.DelayedHandler.handle"));

// Check the content is available
Expand Down Expand Up @@ -469,7 +469,7 @@ public boolean handle(Request request, Response response, Callback callback) thr
ByteArrayOutputStream out = new ByteArrayOutputStream(8192);
new Throwable().printStackTrace(new PrintStream(out));
String stack = out.toString(StandardCharsets.ISO_8859_1);
assertThat(stack, containsString("org.eclipse.jetty.server.internal.HttpConnection.onFillable"));
assertThat(stack, containsString("org.eclipse.jetty.server.HttpConnection.onFillable"));
assertThat(stack, containsString("org.eclipse.jetty.server.handler.DelayedHandler.handle"));

Fields fields = FormFields.from(request).get(1, TimeUnit.NANOSECONDS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,8 @@ protected void customize(Socket socket, Class<? extends Connection> connection,

assertEquals("customize connector class org.eclipse.jetty.io.ssl.SslConnection,false", history.poll());
assertEquals("customize ssl class org.eclipse.jetty.io.ssl.SslConnection,false", history.poll());
assertEquals("customize connector class org.eclipse.jetty.server.internal.HttpConnection,true", history.poll());
assertEquals("customize http class org.eclipse.jetty.server.internal.HttpConnection,true", history.poll());
assertEquals("customize connector class org.eclipse.jetty.server.HttpConnection,true", history.poll());
assertEquals("customize http class org.eclipse.jetty.server.HttpConnection,true", history.poll());
assertEquals(0, history.size());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ protected void customize(Socket socket, Class<? extends Connection> connection,

assertEquals("customize connector class org.eclipse.jetty.io.ssl.SslConnection,false", history.poll());
assertEquals("customize ssl class org.eclipse.jetty.io.ssl.SslConnection,false", history.poll());
assertEquals("customize connector class org.eclipse.jetty.server.internal.HttpConnection,true", history.poll());
assertEquals("customize http class org.eclipse.jetty.server.internal.HttpConnection,true", history.poll());
assertEquals("customize connector class org.eclipse.jetty.server.HttpConnection,true", history.poll());
assertEquals("customize http class org.eclipse.jetty.server.HttpConnection,true", history.poll());
assertEquals(0, history.size());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletException;
import jakarta.servlet.ServletInputStream;
import jakarta.servlet.ServletOutputStream;
import jakarta.servlet.ServletRequest;
import jakarta.servlet.ServletRequestAttributeEvent;
import jakarta.servlet.ServletRequestAttributeListener;
Expand All @@ -56,6 +57,7 @@
import jakarta.servlet.http.HttpUpgradeHandler;
import jakarta.servlet.http.Part;
import jakarta.servlet.http.PushBuilder;
import jakarta.servlet.http.WebConnection;
import org.eclipse.jetty.ee10.servlet.ServletContextHandler.ServletRequestInfo;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.CookieCache;
Expand All @@ -71,12 +73,14 @@
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.SetCookieParser;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.QuietException;
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.security.AuthenticationState;
import org.eclipse.jetty.security.UserIdentity;
import org.eclipse.jetty.server.ConnectionMetaData;
import org.eclipse.jetty.server.FormFields;
import org.eclipse.jetty.server.HttpConnection;
import org.eclipse.jetty.server.HttpCookieUtils;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
Expand Down Expand Up @@ -605,8 +609,94 @@ public Part getPart(String name) throws IOException, ServletException
@Override
public <T extends HttpUpgradeHandler> T upgrade(Class<T> handlerClass) throws IOException, ServletException
{
// Not implemented. Throw ServletException as per spec.
throw new ServletException("Not implemented");
Response response = _servletContextRequest.getServletContextResponse();
if (response.getStatus() != HttpStatus.SWITCHING_PROTOCOLS_101)
throw new IllegalStateException("Response status should be 101");
if (response.getHeaders().get("Upgrade") == null)
throw new IllegalStateException("Missing Upgrade header");
if (!"Upgrade".equalsIgnoreCase(response.getHeaders().get("Connection")))
throw new IllegalStateException("Invalid Connection header");
if (response.isCommitted())
throw new IllegalStateException("Cannot upgrade committed response");
if (_servletChannel.getConnectionMetaData().getHttpVersion() != HttpVersion.HTTP_1_1)
throw new IllegalStateException("Only requests over HTTP/1.1 can be upgraded");

ServletOutputStream outputStream = _servletContextRequest.getHttpOutput();
ServletInputStream inputStream = _servletContextRequest.getHttpInput();
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved

T upgradeHandler;
try
{
upgradeHandler = handlerClass.getDeclaredConstructor().newInstance();
}
catch (Exception e)
{
throw new ServletException("Unable to instantiate handler class", e);
}

HttpConnection httpConnection = (HttpConnection)_servletContextRequest.getConnectionMetaData().getConnection();
httpConnection.getParser().servletUpgrade();
AsyncContext asyncContext = forceStartAsync(); // force the servlet in async mode

outputStream.flush(); // commit the 101 response
httpConnection.getGenerator().servletUpgrade(); // tell the generator it can send data as-is
httpConnection.addEventListener(new Connection.Listener()
{
@Override
public void onClosed(Connection connection)
{
try
{
asyncContext.complete();
}
catch (Exception e)
{
LOG.warn("error during upgrade AsyncContext complete", e);
}
try
{
upgradeHandler.destroy();
}
catch (Exception e)
{
LOG.warn("error during upgrade HttpUpgradeHandler destroy", e);
}
}

@Override
public void onOpened(Connection connection)
{
}
});

upgradeHandler.init(new WebConnection()
{
@Override
public void close() throws Exception
{
try
{
inputStream.close();
}
finally
{
outputStream.close();
}
}

@Override
public ServletInputStream getInputStream()
{
return inputStream;
}

@Override
public ServletOutputStream getOutputStream()
{
return outputStream;
}
});
return upgradeHandler;
}

@Override
Expand Down Expand Up @@ -1230,6 +1320,11 @@ public AsyncContext startAsync() throws IllegalStateException
{
if (!isAsyncSupported())
throw new IllegalStateException("Async Not Supported");
return forceStartAsync();
}

private AsyncContext forceStartAsync()
{
ServletChannelState state = getServletRequestInfo().getState();
if (_async == null)
_async = new AsyncContextState(state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.eclipse.jetty.server.ServerConnector;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -72,7 +71,6 @@ public void tearDown() throws Exception
server.stop();
}

@Disabled
@Test
public void upgradeTest() throws Exception
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@
import org.eclipse.jetty.ee10.servlet.ServletContextRequest;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.server.HttpConnection;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IO;
Expand Down
Loading