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.

Sign up for GitHub

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 and ee11 #12186

Merged
merged 10 commits into from
Sep 4, 2024
Merged

implement servlet upgrade for ee10 and ee11 #12186

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a9d3910
implement servlet upgrade for ee10 and ee11
lachlan-roberts Aug 21, 2024
0e73c56
add missing licence header
lachlan-roberts Aug 22, 2024
cb691cd
PR #12186 - deprecate servletUpgrade method on HttpParser and rename …
lachlan-roberts Aug 23, 2024
594a650
Merge remote-tracking branch 'origin/jetty-12.1.x' into jetty-12.1.x-…
lachlan-roberts Aug 27, 2024
b6f2c86
PR #12186 - Make HttpConnection internal and implement a public Upgra…
lachlan-roberts Aug 29, 2024
e13b9ad
PR #12186 - changes from review
lachlan-roberts Aug 29, 2024
c64e0bb
PR #12186 - add javadoc for Connection.Upgrade
lachlan-roberts Aug 29, 2024
677de2f
Merge remote-tracking branch 'origin/jetty-12.1.x' into jetty-12.1.x-…
lachlan-roberts Aug 29, 2024
9fbde9b
Merge remote-tracking branch 'origin/jetty-12.1.x' into jetty-12.1.x-…
lachlan-roberts Aug 29, 2024
f795fb1
PR # 12186 - changes from review
lachlan-roberts Sep 2, 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
12 changes: 9 additions & 3 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2016,13 +2016,19 @@ public void reset()
_headerComplete = false;
}

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

@Deprecated
public void servletUpgrade()
{
upgrade();
}

protected void setState(State state)
{
if (debugEnabled)
Expand Down
2 changes: 1 addition & 1 deletion ...http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2CServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.RetainableByteBuffer;
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
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,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
1 change: 0 additions & 1 deletion ...-core/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java
View file
Open in desktop
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
13 changes: 2 additions & 11 deletions ...jetty/server/internal/HttpConnection.java → .../eclipse/jetty/server/HttpConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// ========================================================================
//

package org.eclipse.jetty.server.internal;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why this move is necessary?

Is it so that ServletAPIRequest can do:

        HttpConnection httpConnection = (HttpConnection)_servletContextRequest.getConnectionMetaData().getConnection();
        httpConnection.getParser().servletUpgrade();
        outputStream.flush(); // commit the 101 response
        httpConnection.getGenerator().servletUpgrade(); // tell the generator it can send data as-is

We already have interfaces: Connection.UpgradeFrom, Connection.UpgradeTo to abstract away from specific connection types. so can't you use them instead? Get those methods to flip the parser and generator states?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it so that ServletAPIRequest can do:

Yes, this is why it was necessary.

We already have interfaces: Connection.UpgradeFrom, Connection.UpgradeTo to abstract away from specific connection types. so can't you use them instead? Get those methods to flip the parser and generator states?

These interfaces are already implemented on HttpConnection, I think for the HTTP/2 upgrade.

So I think the options are (unless you have a better idea)

  1. make HttpConnection public
  2. create a new Upgrade interface and implement it on the HttpConnection
  3. add a new method to the Connection interface.

Copy link
Contributor

Choose a reason for hiding this comment

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

Option 2

package org.eclipse.jetty.server;

import java.io.IOException;
import java.nio.ByteBuffer;
Expand Down Expand Up @@ -53,16 +53,7 @@
import org.eclipse.jetty.io.RetainableByteBuffer;
import org.eclipse.jetty.io.RuntimeIOException;
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
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,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;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/DelayedServerTest.java
View file
Open in desktop
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
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java
View file
Open in desktop
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
1 change: 0 additions & 1 deletion ...e/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java
View file
Open in desktop
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
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,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
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java
View file
Open in desktop
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
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseCompleteTest.java
View file
Open in desktop
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
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,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.Jetty;
Expand Down
1 change: 0 additions & 1 deletion ...tty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java
View file
Open in desktop
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
1 change: 0 additions & 1 deletion jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/StopTest.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions ...-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DelayedHandlerTest.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions .../jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions ...ore/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...-transports/src/test/java/org/eclipse/jetty/test/client/transport/ConnectionPoolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.quic.server.ServerQuicConnection;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.server.HttpConnection;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

Expand Down
Loading
Loading