HyukjinKwon commented on PR #58640:
URL: https://github.com/apache/spark/pull/58640#issuecomment-5593965281
Thanks for working on this. The approach looks solid, and the test coverage
is great — unit tests plus Selenium coverage for the
GET/POST/HEAD/prefetch/missing-token/wrong-token cases and both render modes.
A couple of points:
1. **Backport compatibility.** The CSRF token requirement is unconditional
(there's no config to disable it), so on the maintenance branches (3.5.10 /
4.0.5 / 4.1.4 / 4.2.1) any programmatic, scripted, or bookmarked caller of the
kill/hold/resume endpoints will start getting `403`s. In-UI usage is unaffected
since the pages embed the token. Given we default `spark.ui.killViaGetEnabled`
off outside YARN and treat this as security hardening, that's reasonable — but
could we call out the behavior change in the release/migration notes for those
maintenance releases so operators aren't surprised?
2. **Minor:** in `JettyUtils.isPrefetchRequest`, `request.getHeader("X-Moz")
!= null` treats any `X-Moz` value as a prefetch, whereas the
`Sec-Purpose`/`Purpose` checks match `"prefetch"` specifically. Matching
`"prefetch"` there too would be a bit more consistent (in practice `X-Moz` is
only ever `prefetch`, so this is cosmetic).
--
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]
