This is an automated email from the ASF dual-hosted git repository.
srowen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 6c19bf6b48e7 [SPARK-45522][BUILD][CORE][SQL][UI] Migrate from Jetty 9
to Jetty 10
6c19bf6b48e7 is described below
commit 6c19bf6b48e7e2ab9937dc2d91ea23dd83abae64
Author: HiuFung Kwok <hiufk...@gmail.com>
AuthorDate: Wed Jan 31 09:42:16 2024 -0600
[SPARK-45522][BUILD][CORE][SQL][UI] Migrate from Jetty 9 to Jetty 10
### What changes were proposed in this pull request?
This is an upgrade ticket to bump the Jetty version from 9 to 10.
This PR aims to bring incremental Jetty upgrades to Spark, as Jetty 9
support already reached EOL.
### Why are the changes needed?
Jetty 9 is already beyond EOL, which means that we won't receive any
security fix onward for Spark.
### Does this PR introduce _any_ user-facing change?
No, SNI host check is now defaulted to true on embedded Jetty, hence set it
back to false to maintain backward compatibility.
Despite the redirect behaviour changed for trailing /, but modern browser
should be able to pick up the 302 status code and perform redirect accordingly,
hence there is no impact on user level.
### How was this patch tested?
Junit test case.
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #43765 from HiuKwok/ft-hf-SPARK-45522-jetty-upgradte.
Lead-authored-by: HiuFung Kwok <hiufk...@gmail.com>
Co-authored-by: HiuFung Kwok <37996731+hiuk...@users.noreply.github.com>
Signed-off-by: Sean Owen <sro...@gmail.com>
---
LICENSE-binary | 1 -
core/pom.xml | 8 +-------
.../main/scala/org/apache/spark/SSLOptions.scala | 2 +-
.../main/scala/org/apache/spark/TestUtils.scala | 13 +++++++++++++
.../scala/org/apache/spark/ui/JettyUtils.scala | 13 ++++++++++---
.../test/scala/org/apache/spark/ui/UISuite.scala | 22 +++++++++++++++++-----
dev/deps/spark-deps-hadoop-3-hive-2.3 | 4 ++--
dev/test-dependencies.sh | 2 +-
pom.xml | 8 +-------
.../service/cli/thrift/ThriftHttpCLIService.java | 12 ++++++------
10 files changed, 52 insertions(+), 33 deletions(-)
diff --git a/LICENSE-binary b/LICENSE-binary
index c6f291f11088..2073d85246b6 100644
--- a/LICENSE-binary
+++ b/LICENSE-binary
@@ -368,7 +368,6 @@ xerces:xercesImpl
org.codehaus.jackson:jackson-jaxrs
org.codehaus.jackson:jackson-xc
org.eclipse.jetty:jetty-client
-org.eclipse.jetty:jetty-continuation
org.eclipse.jetty:jetty-http
org.eclipse.jetty:jetty-io
org.eclipse.jetty:jetty-jndi
diff --git a/core/pom.xml b/core/pom.xml
index c093213bd6b9..f780551fb555 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -146,11 +146,6 @@
<artifactId>jetty-http</artifactId>
<scope>compile</scope>
</dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-continuation</artifactId>
- <scope>compile</scope>
- </dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-servlet</artifactId>
@@ -538,7 +533,7 @@
<overWriteIfNewer>true</overWriteIfNewer>
<useSubDirectoryPerType>true</useSubDirectoryPerType>
<includeArtifactIds>
-
guava,protobuf-java,jetty-io,jetty-servlet,jetty-servlets,jetty-continuation,jetty-http,jetty-plus,jetty-util,jetty-server,jetty-security,jetty-proxy,jetty-client
+
guava,protobuf-java,jetty-io,jetty-servlet,jetty-servlets,jetty-http,jetty-plus,jetty-util,jetty-server,jetty-security,jetty-proxy,jetty-client
</includeArtifactIds>
<silent>true</silent>
</configuration>
@@ -558,7 +553,6 @@
<include>org.eclipse.jetty:jetty-http</include>
<include>org.eclipse.jetty:jetty-proxy</include>
<include>org.eclipse.jetty:jetty-client</include>
- <include>org.eclipse.jetty:jetty-continuation</include>
<include>org.eclipse.jetty:jetty-servlet</include>
<include>org.eclipse.jetty:jetty-servlets</include>
<include>org.eclipse.jetty:jetty-plus</include>
diff --git a/core/src/main/scala/org/apache/spark/SSLOptions.scala
b/core/src/main/scala/org/apache/spark/SSLOptions.scala
index 26108d885e4c..ce058cec2686 100644
--- a/core/src/main/scala/org/apache/spark/SSLOptions.scala
+++ b/core/src/main/scala/org/apache/spark/SSLOptions.scala
@@ -87,7 +87,7 @@ private[spark] case class SSLOptions(
/**
* Creates a Jetty SSL context factory according to the SSL settings
represented by this object.
*/
- def createJettySslContextFactory(): Option[SslContextFactory] = {
+ def createJettySslContextFactoryServer(): Option[SslContextFactory.Server] =
{
if (enabled) {
val sslContextFactory = new SslContextFactory.Server()
diff --git a/core/src/main/scala/org/apache/spark/TestUtils.scala
b/core/src/main/scala/org/apache/spark/TestUtils.scala
index e85f98ff55c5..5e3078d7292b 100644
--- a/core/src/main/scala/org/apache/spark/TestUtils.scala
+++ b/core/src/main/scala/org/apache/spark/TestUtils.scala
@@ -252,6 +252,19 @@ private[spark] object TestUtils extends SparkTestUtils {
}
}
+ /**
+ * Returns the Location header from an HTTP(S) URL.
+ */
+ def redirectUrl(
+ url: URL,
+ method: String = "GET",
+ headers: Seq[(String, String)] = Nil): String = {
+ withHttpConnection(url, method, headers = headers) { connection =>
+ connection.getHeaderField("Location");
+ }
+ }
+
+
/**
* Returns the response message from an HTTP(S) URL.
*/
diff --git a/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
b/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
index 849ee14c0afb..a0f65606e60d 100644
--- a/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
+++ b/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
@@ -204,7 +204,7 @@ private[spark] object JettyUtils extends Logging {
// SPARK-21176: Use the Jetty logic to calculate the number of
selector threads (#CPUs/2),
// but limit it to 8 max.
val numSelectors = math.max(1, math.min(8,
Runtime.getRuntime().availableProcessors() / 2))
- new HttpClient(new HttpClientTransportOverHTTP(numSelectors), null)
+ new HttpClient(new HttpClientTransportOverHTTP(numSelectors))
}
override def filterServerResponseHeader(
@@ -300,7 +300,6 @@ private[spark] object JettyUtils extends Logging {
connector.setReuseAddress(!Utils.isWindows)
// spark-45248: set the idle timeout to prevent slow DoS
connector.setIdleTimeout(8000)
- connector.setStopTimeout(stopTimeout)
// Currently we only use "SelectChannelConnector"
// Limit the max acceptor number to 8 so that we don't waste a lot of
threads
@@ -324,9 +323,17 @@ private[spark] object JettyUtils extends Logging {
httpConfig.setSendXPoweredBy(false)
// If SSL is configured, create the secure connector first.
- val securePort = sslOptions.createJettySslContextFactory().map { factory
=>
+ val securePort = sslOptions.createJettySslContextFactoryServer().map {
factory =>
+
+ // SPARK-45522: SniHostCheck defaulted to true since Jetty 10,
+ // this will affect the standalone deployment.
+ val src = new SecureRequestCustomizer()
+ src.setSniHostCheck(false)
+ httpConfig.addCustomizer(src)
+
val securePort = sslOptions.port.getOrElse(if (port > 0)
Utils.userPort(port, 400) else 0)
val secureServerName = if (serverName.nonEmpty) s"$serverName (HTTPS)"
else serverName
+
val connectionFactories =
AbstractConnectionFactory.getFactories(factory,
new HttpConnectionFactory(httpConfig))
diff --git a/core/src/test/scala/org/apache/spark/ui/UISuite.scala
b/core/src/test/scala/org/apache/spark/ui/UISuite.scala
index e7d57a6e6def..cb5fbda3e58a 100644
--- a/core/src/test/scala/org/apache/spark/ui/UISuite.scala
+++ b/core/src/test/scala/org/apache/spark/ui/UISuite.scala
@@ -359,9 +359,20 @@ class UISuite extends SparkFunSuite {
try {
val serverAddr = s"http://$localhost:${serverInfo.boundPort}";
+ val (_, ctx) = newContext("/ctx1")
+ serverInfo.addHandler(ctx, securityMgr)
+
val redirect = JettyUtils.createRedirectHandler("/src", "/dst")
serverInfo.addHandler(redirect, securityMgr)
+ // Test Jetty's built-in redirect to add the trailing slash to the
context path.
+ TestUtils.withHttpConnection(new URL(s"$serverAddr/ctx1")) { conn =>
+ assert(conn.getResponseCode() === HttpServletResponse.SC_FOUND)
+ val location = Option(conn.getHeaderFields().get("Location"))
+ .map(_.get(0)).orNull
+ assert(location === s"$proxyRoot/ctx1/")
+ }
+
// Test with a URL handled by the added redirect handler, and also
including a path prefix.
val headers = Seq("X-Forwarded-Context" -> "/prefix")
TestUtils.withHttpConnection(
@@ -387,8 +398,8 @@ class UISuite extends SparkFunSuite {
}
}
- test("SPARK-34449: Jetty 9.4.35.v20201120 and later no longer return status
code 302 " +
- " and handle internally when request URL ends with a context path
without trailing '/'") {
+ test("SPARK-45522: Jetty 10 and above shouuld return status code 302 with
correct redirect url" +
+ " when request URL ends with a context path without trailing '/'") {
val proxyRoot = "https://proxy.example.com:443/prefix";
val (conf, securityMgr, sslOptions) = sslDisabledConf()
conf.set(UI.PROXY_REDIRECT_URI, proxyRoot)
@@ -401,9 +412,10 @@ class UISuite extends SparkFunSuite {
assert(TestUtils.httpResponseCode(new URL(urlStr + "/")) ===
HttpServletResponse.SC_OK)
- // If the following assertion fails when we upgrade Jetty, it seems to
change the behavior of
- // handling context path which doesn't have the trailing slash.
- assert(TestUtils.httpResponseCode(new URL(urlStr)) ===
HttpServletResponse.SC_OK)
+ // In the case of trailing slash,
+ // 302 should be return and the redirect URL shouuld be part of the
header.
+ assert(TestUtils.redirectUrl(new URL(urlStr)) === proxyRoot + "/ctx/");
+ assert(TestUtils.httpResponseCode(new URL(urlStr)) ===
HttpServletResponse.SC_FOUND)
} finally {
stopServer(serverInfo)
}
diff --git a/dev/deps/spark-deps-hadoop-3-hive-2.3
b/dev/deps/spark-deps-hadoop-3-hive-2.3
index 06fb4d879db2..fcb3350e5de2 100644
--- a/dev/deps/spark-deps-hadoop-3-hive-2.3
+++ b/dev/deps/spark-deps-hadoop-3-hive-2.3
@@ -133,8 +133,8 @@
jersey-container-servlet/2.41//jersey-container-servlet-2.41.jar
jersey-hk2/2.41//jersey-hk2-2.41.jar
jersey-server/2.41//jersey-server-2.41.jar
jettison/1.5.4//jettison-1.5.4.jar
-jetty-util-ajax/9.4.53.v20231009//jetty-util-ajax-9.4.53.v20231009.jar
-jetty-util/9.4.53.v20231009//jetty-util-9.4.53.v20231009.jar
+jetty-util-ajax/10.0.19//jetty-util-ajax-10.0.19.jar
+jetty-util/10.0.19//jetty-util-10.0.19.jar
jline/2.14.6//jline-2.14.6.jar
jline/3.22.0//jline-3.22.0.jar
jna/5.13.0//jna-5.13.0.jar
diff --git a/dev/test-dependencies.sh b/dev/test-dependencies.sh
index 855572dbc637..175f59a70094 100755
--- a/dev/test-dependencies.sh
+++ b/dev/test-dependencies.sh
@@ -51,7 +51,7 @@ OLD_VERSION=$($MVN -q \
# dependency:get for guava and jetty-io are workaround for SPARK-37302.
GUAVA_VERSION=$(build/mvn help:evaluate -Dexpression=guava.version -q
-DforceStdout | grep -E "^[0-9.]+$")
build/mvn dependency:get -Dartifact=com.google.guava:guava:${GUAVA_VERSION} -q
-JETTY_VERSION=$(build/mvn help:evaluate -Dexpression=jetty.version -q
-DforceStdout | grep -E "^[0-9.]+v[0-9]+")
+JETTY_VERSION=$(build/mvn help:evaluate -Dexpression=jetty.version -q
-DforceStdout | grep -E "[0-9]+\.[0-9]+\.[0-9]+")
build/mvn dependency:get
-Dartifact=org.eclipse.jetty:jetty-io:${JETTY_VERSION} -q
if [ $? != 0 ]; then
echo -e "Error while getting version string from Maven:\n$OLD_VERSION"
diff --git a/pom.xml b/pom.xml
index b78f49499feb..6e118bb27f5a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -143,7 +143,7 @@
<parquet.version>1.13.1</parquet.version>
<orc.version>1.9.2</orc.version>
<orc.classifier>shaded-protobuf</orc.classifier>
- <jetty.version>9.4.53.v20231009</jetty.version>
+ <jetty.version>10.0.19</jetty.version>
<jakartaservlet.version>4.0.3</jakartaservlet.version>
<chill.version>0.10.0</chill.version>
<!--
@@ -489,12 +489,6 @@
<version>${jetty.version}</version>
<scope>provided</scope>
</dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-continuation</artifactId>
- <version>${jetty.version}</version>
- <scope>provided</scope>
- </dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-servlet</artifactId>
diff --git
a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java
b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java
index 140a3fb8954d..3fe60aa681f9 100644
---
a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java
+++
b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java
@@ -84,16 +84,16 @@ public class ThriftHttpCLIService extends ThriftCLIService {
throw new
IllegalArgumentException(ConfVars.HIVE_SERVER2_SSL_KEYSTORE_PATH.varname
+ " Not configured for SSL connection");
}
- SslContextFactory sslContextFactory = new SslContextFactory.Server();
+ SslContextFactory.Server sslContextFactoryServer = new
SslContextFactory.Server();
String[] excludedProtocols =
hiveConf.getVar(ConfVars.HIVE_SSL_PROTOCOL_BLACKLIST).split(",");
LOG.info("HTTP Server SSL: adding excluded protocols: " +
Arrays.toString(excludedProtocols));
- sslContextFactory.addExcludeProtocols(excludedProtocols);
+ sslContextFactoryServer.addExcludeProtocols(excludedProtocols);
LOG.info("HTTP Server SSL: SslContextFactory.getExcludeProtocols = " +
- Arrays.toString(sslContextFactory.getExcludeProtocols()));
- sslContextFactory.setKeyStorePath(keyStorePath);
- sslContextFactory.setKeyStorePassword(keyStorePassword);
+ Arrays.toString(sslContextFactoryServer.getExcludeProtocols()));
+ sslContextFactoryServer.setKeyStorePath(keyStorePath);
+ sslContextFactoryServer.setKeyStorePassword(keyStorePassword);
connectionFactories = AbstractConnectionFactory.getFactories(
- sslContextFactory, new HttpConnectionFactory());
+ sslContextFactoryServer, new HttpConnectionFactory());
} else {
connectionFactories = new ConnectionFactory[] { new
HttpConnectionFactory() };
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
