-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-47086][BUILD][CORE][SQL][UI] Migrate from Jetty 11 to Jetty 12 #45500
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,15 +46,16 @@ import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFact | |
| import org.eclipse.jetty.server.Handler | ||
| import org.eclipse.jetty.server.Server | ||
| import org.eclipse.jetty.server.handler.DefaultHandler | ||
| import org.eclipse.jetty.server.handler.HandlerList | ||
| import org.eclipse.jetty.server.handler.ResourceHandler | ||
| import org.eclipse.jetty.util.resource.ResourceFactory | ||
| import org.json4s.JsonAST.JValue | ||
| import org.json4s.jackson.JsonMethods.{compact, render} | ||
|
|
||
| import org.apache.spark.executor.TaskMetrics | ||
| import org.apache.spark.scheduler._ | ||
| import org.apache.spark.util.{SparkTestUtils, Utils} | ||
|
|
||
|
|
||
| /** | ||
| * Utilities for tests. Included in main codebase since it's used by multiple | ||
| * projects. | ||
|
|
@@ -335,9 +336,9 @@ private[spark] object TestUtils extends SparkTestUtils { | |
| // 0 as port means choosing randomly from the available ports | ||
| val server = new Server(new InetSocketAddress(Utils.localCanonicalHostName(), 0)) | ||
| val resHandler = new ResourceHandler() | ||
| resHandler.setResourceBase(resBaseDir) | ||
| val handlers = new HandlerList() | ||
| handlers.setHandlers(Array[Handler](resHandler, new DefaultHandler())) | ||
| resHandler.setBaseResource(ResourceFactory.of(resHandler).newResource(resBaseDir)) | ||
| val handlers = new Handler.Sequence; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please follow the Scala style. We don't need |
||
| handlers.setHandlers(resHandler, new DefaultHandler()) | ||
| server.setHandler(handlers) | ||
| server.start() | ||
| try { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,22 +18,25 @@ | |
| package org.apache.spark.ui | ||
|
|
||
| import java.net.{URI, URL, URLDecoder} | ||
| import java.nio.charset.Charset | ||
| import java.util.EnumSet | ||
|
|
||
| import scala.jdk.CollectionConverters._ | ||
| import scala.language.implicitConversions | ||
| import scala.util.Try | ||
| import scala.xml.Node | ||
|
|
||
| import jakarta.servlet.DispatcherType | ||
| import jakarta.servlet.http._ | ||
| import org.eclipse.jetty.client.HttpClient | ||
| import org.eclipse.jetty.client.api.Response | ||
| import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP | ||
| import org.eclipse.jetty.proxy.ProxyServlet | ||
| import org.eclipse.jetty.client.transport.HttpClientTransportOverHTTP | ||
| import org.eclipse.jetty.ee10.proxy.ProxyServlet | ||
| import org.eclipse.jetty.ee10.servlet.{DefaultServlet, FilterHolder, ServletContextHandler, ServletHolder} | ||
| import org.eclipse.jetty.http.HttpHeader | ||
| import org.eclipse.jetty.server._ | ||
| import org.eclipse.jetty.server.handler._ | ||
| import org.eclipse.jetty.server.handler.gzip.GzipHandler | ||
| import org.eclipse.jetty.servlet._ | ||
| import org.eclipse.jetty.util.{Callback, UrlEncoded} | ||
| import org.eclipse.jetty.util.component.LifeCycle | ||
| import org.eclipse.jetty.util.thread.{QueuedThreadPool, ScheduledExecutorScheduler} | ||
| import org.json4s.JValue | ||
|
|
@@ -149,6 +152,7 @@ private[spark] object JettyUtils extends Logging { | |
| // Make sure we don't end up with "//" in the middle | ||
| val newUrl = new URL(new URL(request.getRequestURL.toString), prefixedDestPath).toString | ||
| response.sendRedirect(newUrl) | ||
| // Response.sendRedirect(request, response, callback, location) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please clean up this leftover. |
||
| } | ||
| // SPARK-5983 ensure TRACE is not supported | ||
| protected override def doTrace(req: HttpServletRequest, res: HttpServletResponse): Unit = { | ||
|
|
@@ -209,12 +213,12 @@ private[spark] object JettyUtils extends Logging { | |
|
|
||
| override def filterServerResponseHeader( | ||
| clientRequest: HttpServletRequest, | ||
| serverResponse: Response, | ||
| serverResponse: org.eclipse.jetty.client.Response, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use |
||
| headerName: String, | ||
| headerValue: String): String = { | ||
| if (headerName.equalsIgnoreCase("location")) { | ||
| val newHeader = createProxyLocationHeader(headerValue, clientRequest, | ||
| serverResponse.getRequest().getURI()) | ||
| serverResponse.getRequest.getURI) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this change really? Otherwise, please revert all this kind of style change. |
||
| if (newHeader != null) { | ||
| return newHeader | ||
| } | ||
|
|
@@ -259,7 +263,7 @@ private[spark] object JettyUtils extends Logging { | |
|
|
||
| val errorHandler = new ErrorHandler() | ||
| errorHandler.setShowStacks(true) | ||
| errorHandler.setServer(server) | ||
| server.setErrorHandler(errorHandler); | ||
| server.addBean(errorHandler) | ||
|
|
||
| val collection = new ContextHandlerCollection | ||
|
|
@@ -387,20 +391,19 @@ private[spark] object JettyUtils extends Logging { | |
| private def createRedirectHttpsHandler(securePort: Int, scheme: String): ContextHandler = { | ||
| val redirectHandler: ContextHandler = new ContextHandler | ||
| redirectHandler.setContextPath("/") | ||
| redirectHandler.setVirtualHosts(toVirtualHosts(REDIRECT_CONNECTOR_NAME)) | ||
| redirectHandler.setHandler(new AbstractHandler { | ||
| redirectHandler.setVirtualHosts(toVirtualHosts(REDIRECT_CONNECTOR_NAME).asJava) | ||
| redirectHandler.setHandler(new Handler.Abstract { | ||
| override def handle( | ||
| target: String, | ||
| baseRequest: Request, | ||
| request: HttpServletRequest, | ||
| response: HttpServletResponse): Unit = { | ||
| if (baseRequest.isSecure) { | ||
| return | ||
| } | ||
| val httpsURI = createRedirectURI(scheme, securePort, baseRequest) | ||
| response.setContentLength(0) | ||
| response.sendRedirect(response.encodeRedirectURL(httpsURI)) | ||
| baseRequest.setHandled(true) | ||
| request: Request, | ||
| response: Response, | ||
| callback: Callback): Boolean = { | ||
| if (request.isSecure) return false | ||
| val httpsURI = createRedirectURI(scheme, securePort, request) | ||
| val responseHeaders = response.getHeaders | ||
| responseHeaders.put(HttpHeader.CONTENT_LENGTH, 0L) | ||
| val location = Response.toRedirectURI(request, httpsURI); | ||
| Response.sendRedirect(request, response, callback, location) | ||
| true | ||
| } | ||
| }) | ||
| redirectHandler | ||
|
|
@@ -455,7 +458,7 @@ private[spark] object JettyUtils extends Logging { | |
| handler.addFilter(holder, "/*", EnumSet.allOf(classOf[DispatcherType])) | ||
| } | ||
|
|
||
| private def decodeURL(url: String, encoding: String): String = { | ||
| private def decodeURL(url: String, encoding: Charset): String = { | ||
| if (url == null) { | ||
| null | ||
| } else { | ||
|
|
@@ -465,27 +468,20 @@ private[spark] object JettyUtils extends Logging { | |
|
|
||
| // Create a new URI from the arguments, handling IPv6 host encoding and default ports. | ||
| private def createRedirectURI(scheme: String, port: Int, request: Request): String = { | ||
| val server = request.getServerName | ||
| val server = Request.getServerName(request) | ||
| val redirectServer = if (server.contains(":") && !server.startsWith("[")) { | ||
| s"[${server}]" | ||
| } else { | ||
| server | ||
| } | ||
| request.getHttpURI.getDecodedPath | ||
| val authority = s"$redirectServer:$port" | ||
| val queryEncoding = if (request.getQueryEncoding != null) { | ||
| request.getQueryEncoding | ||
| } else { | ||
| // By default decoding the URI as "UTF-8" should be enough for SparkUI | ||
| "UTF-8" | ||
| } | ||
| // The request URL can be raw or encoded here. To avoid the request URL being | ||
| // encoded twice, let's decode it here. | ||
| val requestURI = decodeURL(request.getRequestURI, queryEncoding) | ||
| val queryString = decodeURL(request.getQueryString, queryEncoding) | ||
| val requestURI = request.getHttpURI.getDecodedPath | ||
| val queryString = decodeURL(request.getHttpURI.getQuery, UrlEncoded.ENCODING) | ||
| new URI(scheme, authority, requestURI, queryString, null).toString | ||
| } | ||
|
|
||
| def toVirtualHosts(connectors: String*): Array[String] = connectors.map("@" + _).toArray | ||
| def toVirtualHosts(connectors: String*): List[String] = connectors.map("@" + _).toList | ||
|
|
||
| } | ||
|
|
||
|
|
@@ -499,7 +495,7 @@ private[spark] case class ServerInfo( | |
| def addHandler( | ||
| handler: ServletContextHandler, | ||
| securityMgr: SecurityManager): Unit = synchronized { | ||
| handler.setVirtualHosts(JettyUtils.toVirtualHosts(JettyUtils.SPARK_CONNECTOR_NAME)) | ||
| handler.setVirtualHosts(JettyUtils.toVirtualHosts(JettyUtils.SPARK_CONNECTOR_NAME).asJava) | ||
| addFilters(handler, securityMgr) | ||
|
|
||
| val gzipHandler = new GzipHandler() | ||
|
|
@@ -515,7 +511,7 @@ private[spark] case class ServerInfo( | |
| def removeHandler(handler: ServletContextHandler): Unit = synchronized { | ||
| // Since addHandler() always adds a wrapping gzip handler, find the container handler | ||
| // and remove it. | ||
| rootHandler.getHandlers() | ||
| rootHandler.getHandlers.asScala | ||
| .find { h => | ||
| h.isInstanceOf[GzipHandler] && h.asInstanceOf[GzipHandler].getHandler() == handler | ||
| } | ||
|
|
@@ -579,6 +575,12 @@ private[spark] case class ServerInfo( | |
|
|
||
| } | ||
|
|
||
| // private def getRedirectUrl(location: String): Unit = { | ||
| // | ||
| // val proxyUri = _proxyUri.stripSuffix("/") | ||
| // | ||
| // } | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please clean up this left-over. |
||
| /** | ||
| * A Jetty handler to handle redirects to a proxy server. It intercepts redirects and rewrites the | ||
| * location to point to the proxy server. | ||
|
|
@@ -588,36 +590,37 @@ private[spark] case class ServerInfo( | |
| * a servlet context without the trailing slash (e.g. "/jobs") - Jetty will send a redirect to the | ||
| * same URL, but with a trailing slash. | ||
| */ | ||
| private class ProxyRedirectHandler(_proxyUri: String) extends HandlerWrapper { | ||
| private class ProxyRedirectHandler(_proxyUri: String) extends Handler.Wrapper { | ||
|
|
||
| private val proxyUri = _proxyUri.stripSuffix("/") | ||
|
|
||
| override def handle( | ||
| target: String, | ||
| baseRequest: Request, | ||
| request: HttpServletRequest, | ||
| response: HttpServletResponse): Unit = { | ||
| super.handle(target, baseRequest, request, new ResponseWrapper(request, response)) | ||
| } | ||
|
|
||
| private class ResponseWrapper( | ||
| req: HttpServletRequest, | ||
| res: HttpServletResponse) | ||
| extends HttpServletResponseWrapper(res) { | ||
|
|
||
| override def sendRedirect(location: String): Unit = { | ||
| val newTarget = if (location != null) { | ||
| val target = new URI(location) | ||
| // The target path should already be encoded, so don't re-encode it, just the | ||
| // proxy address part. | ||
| val proxyBase = UIUtils.uiRoot(req) | ||
| val proxyPrefix = if (proxyBase.nonEmpty) s"$proxyUri$proxyBase" else proxyUri | ||
| s"${res.encodeURL(proxyPrefix)}${target.getPath()}" | ||
| } else { | ||
| null | ||
| } | ||
| super.sendRedirect(newTarget) | ||
| } | ||
| request: Request, | ||
| response: org.eclipse.jetty.server.Response, | ||
| callback: Callback): Boolean = { | ||
| // Todo: Fix the proxy redirect behaviour. | ||
| // super.handle(request, new ResponseWrapper(request, response), callback) | ||
| super.handle(request, response, callback) | ||
| } | ||
| // | ||
| // private class ResponseWrapper( | ||
| // req: Request, | ||
| // res: Response) | ||
| // extends Response.Wrapper(req, res) { | ||
| // | ||
| // override def sendRedirect(location: String): Unit = { | ||
| // val newTarget = if (location != null) { | ||
| // val target = new URI(location) | ||
| // // The target path should already be encoded, so don't re-encode it, just the | ||
| // // proxy address part. | ||
| // val proxyBase = UIUtils.uiRoot(req) | ||
| // val proxyPrefix = if (proxyBase.nonEmpty) s"$proxyUri$proxyBase" else proxyUri | ||
| // s"${res.encodeURL(proxyPrefix)}${target.getPath()}" | ||
| // } else { | ||
| // null | ||
| // } | ||
| // super.sendRedirect(newTarget) | ||
| // } | ||
| // } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove this leftover. |
||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this kind of empty lines to minimize your PR.