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