yadavay-amzn opened a new pull request, #56528:
URL: https://github.com/apache/spark/pull/56528
### What changes were proposed in this pull request?
This PR moves the
`spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties` toggle out of
per-session `HiveConf` and onto a static field on `HiveSessionImpl`, populated
once at server init by `SparkSQLSessionManager`.
### Why are the changes needed?
SPARK-57441 added a default-deny gate on `set:system:*` in
`HiveSessionImpl.setVariable`, but read its toggle from
`ss.getConf().getBoolean(...)` -- the per-session `HiveConf`. The same
`setVariable` method writes to that same `HiveConf` from its `HIVECONF_PREFIX`
branch via `setConf` -> `verifyAndSet`, with no validation that rejects
`spark.*` keys (Hive's `getConfVars` returns null for non-Hive keys, the
`key.startsWith("hive.")` fallback does not fire, and `verifyAndSet` has no
default `restrictList` entry for this key).
A JDBC URL of the form
```
jdbc:hive2://host:port/db?set:hiveconf:spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties=true;set:system:foo=bar
```
flips the flag in the per-session `HiveConf` via the first directive, then
the second directive's gate read sees `true` and proceeds with
`System.setProperty`, bypassing the default-deny. See
[SPARK-57480](https://issues.apache.org/jira/browse/SPARK-57480) for the repro
and impact analysis.
The fix detaches the gate read from per-session state. The static field is
set once at `SparkSQLSessionManager.init` from the server-wide `StaticSQLConf`,
and is no longer reachable from any per-session `set:` overlay. The
`set:hiveconf:<flag-key>=true` write still succeeds in writing into the
per-session `HiveConf` (preserved for backward compatibility), but it is now a
no-op for the gate.
### Does this PR introduce _any_ user-facing change?
No behavior change for users. The default-deny posture from SPARK-57441 is
preserved, and the legacy escape-hatch
(`spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties=true` in the
server's static SQLConf) still works.
### How was this patch tested?
Added `HiveSessionImplSuite` regression test that exercises the exact bypass:
1. Open a session with the legacy flag explicitly disabled.
2. Call `setVariable("hiveconf:<flag-key>", "true")` -- this is what
`configureSession` does for `set:hiveconf:` in a JDBC URL.
3. Call `setVariable("system:<prop>", "<val>")`.
4. Assert that step 3 returns `1` (rejected) and `System.getProperty` is
null.
I verified the regression test fails when the production fix is reverted
(gate read goes back to `ss.getConf().getBoolean(...)`): test reports `0 did
not equal 1 system:* should still be rejected, got rc=0`.
All four `HiveSessionImplSuite` tests pass with the fix in place.
cc @dongjoon-hyun (author of SPARK-57441 / #56501), @gengliangwang
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.7
--
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]
