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

Open
wants to merge 15 commits into
base: jetty-12.0.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1922,8 +1922,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 @@ -27,7 +27,6 @@
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.QuotedCSVParser;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.HostPort;
import org.eclipse.jetty.util.Index;
import org.eclipse.jetty.util.StringUtil;
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.net.SocketAddress;
Expand Down Expand Up @@ -57,15 +57,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.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 Expand Up @@ -1096,18 +1088,17 @@ public void earlyEOF()
HttpStreamOverHTTP1 stream = _stream.get();
if (stream != null)
{
BadMessageException bad = new BadMessageException("Early EOF");

BadMessageException eof = new BadMessageException("Early EOF");
if (Content.Chunk.isFailure(stream._chunk))
stream._chunk.getFailure().addSuppressed(bad);
stream._chunk.getFailure().addSuppressed(eof);
else
{
if (stream._chunk != null)
stream._chunk.release();
stream._chunk = Content.Chunk.from(bad);
stream._chunk = Content.Chunk.from(eof);
}

Runnable todo = _httpChannel.onFailure(bad);
Runnable todo = _httpChannel.onFailure(eof);
if (todo != null)
getServer().getThreadPool().execute(todo);
}
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 @@ -51,6 +51,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;
import org.eclipse.jetty.server.HttpStream;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.RequestLog;
Expand Down Expand Up @@ -1272,7 +1273,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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot have an execute on the callback of every write. This is a blocker. Let's analyse after 12.0.1 and see how we can avoid t his.

}
}

/**
Expand Down Expand Up @@ -1300,7 +1306,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));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbordet I don't like this, but we have a test which calls write from within the callback which results in recursion and a StackOverflowError if we execute the callback directly.

}
}

@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 @@ -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 @@ -24,7 +24,6 @@
import java.util.Arrays;

import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.TypeUtil;
import org.junit.jupiter.api.AfterEach;
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 @@ -33,7 +33,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 @@ -489,8 +489,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 @@ -70,12 +72,14 @@
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MimeTypes;
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 @@ -591,8 +595,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();

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 @@ -1207,6 +1297,11 @@ public AsyncContext startAsync() throws IllegalStateException
{
if (!isAsyncSupported())
throw new IllegalStateException("Async Not Supported");
return forceStartAsync();
}

private AsyncContext forceStartAsync()
{
ServletRequestState state = getServletRequestInfo().getState();
if (_async == null)
_async = new AsyncContextState(state);
Expand Down