Nflrijal commented on PR #59089: URL: https://github.com/apache/spark/pull/59089#issuecomment-6556396213
Hi @HyukjinKwon, Thank you for the detailed review feedback! I have updated the PR (and the PR title/description) to address all findings: 1. **Avoid doubling `X-Forwarded-Context` prefix (`JettyUtils.scala`)**: Extracted `reverseProxyPath` once in `createProxyHandler` using `URI.getRawPath`, and used the incoming `X-Forwarded-Context` header when present instead of prepending `reverseProxyPath` a second time. 2. **Avoid doubling redirect `Location` prefix with `spark.ui.proxyRedirectUri` (`JettyUtils.scala`, `MasterWebUI.scala`, `UI.scala`, `docs/configuration.md`)**: Passed `spark.ui.proxyRedirectUri` from `MasterWebUI.addProxy` to `JettyUtils.createProxyHandler` and only re-rooted proxied `Location` headers onto `spark.ui.reverseProxyUrl` when `spark.ui.proxyRedirectUri` is not set. Updated `spark.ui.reverseProxyUrl` documentation to clarify redirect rewriting responsibilities. 3. **End-to-end proxy handler tests (`UISuite.scala`, `MasterWorkerUISuite.scala`, `JettyUtilsSuite.scala`)**: Removed the mock-only test in `JettyUtilsSuite` and added real HTTP proxy tests in `UISuite` (covering absolute/relative `reverseProxyUrl`, pre-existing `X-Forwarded-Context`, and `spark.ui.proxyRedirectUri`) and `MasterWorkerUISuite` (asserting the 301 redirect `Location` on `/proxy/<workerId>/json`). 4. **App-specific proxy base guard test (`SparkContextSuite.scala`)**: Updated `avoid inheriting previous application-specific proxy path` to test all four recognized ID prefixes (`/proxy/application_`, `/proxy/app-`, `/proxy/spark-`, and `/proxy/local-`) on both start and `stop()`. All `core`, Spark UI, linter, and doc generation CI checks pass (the `StreamingSQLQueryTestSuite` failure in `sql - extended tests` is an unrelated upstream SQL failure). Whenever you have a moment, could you please take another look? Thank you! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
