dprmfl commented on code in PR #19231:
URL: https://github.com/apache/druid/pull/19231#discussion_r3238868432
##########
sql/src/main/java/org/apache/druid/sql/avatica/DruidMeta.java:
##########
@@ -804,6 +816,11 @@ private DruidConnection openDruidConnection(
final Map<String, Object> context
)
{
+ String remoteAddress = THREAD_LOCAL_REMOTE_ADDRESS.get();
+ if (remoteAddress != null) {
+ context.put("remoteAddress", remoteAddress);
Review Comment:
https://github.com/apache/druid/pull/19231/commits/aefdd6421cd34ead7b4991e7107611b081c87380
Thanks for the review. I've addressed this by passing remoteAddress as a
dedicated parameter through the call chain instead of injecting it into the
session context map.
**Before**:
- DruidMeta.openConnection() puts remoteAddress into the session context
map
- SqlQueryPlus.withContext() picks it up as an authContextKey
- With authorizeQueryContextParams enabled, this triggers a WRITE
permission check on QUERY_CONTEXT:remoteAddress
**After**:
- remoteAddress is removed from the session context map entirely
- Passed as a separate constructor/method parameter: DruidMeta →
DruidConnection → DruidJdbcStatement / PreparedStatement → SqlStatementFactory
→ DirectStatement
- No longer treated as a user-supplied context key, so no unintended auth
checks
This keeps it out of authContextKey entirely while still being available
for metrics/logging. Verified by building only the druid-sql jar and swapping
it into a running
cluster — works as expected.
![Uploading image.png…]()
(test version: 37.0.0)
##########
sql/src/main/java/org/apache/druid/sql/avatica/DruidMeta.java:
##########
@@ -804,6 +816,11 @@ private DruidConnection openDruidConnection(
final Map<String, Object> context
)
{
+ String remoteAddress = THREAD_LOCAL_REMOTE_ADDRESS.get();
+ if (remoteAddress != null) {
+ context.put("remoteAddress", remoteAddress);
Review Comment:
https://github.com/apache/druid/pull/19231/commits/aefdd6421cd34ead7b4991e7107611b081c87380
Thanks for the review. I've addressed this by passing remoteAddress as a
dedicated parameter through the call chain instead of injecting it into the
session context map.
**Before**:
- DruidMeta.openConnection() puts remoteAddress into the session context
map
- SqlQueryPlus.withContext() picks it up as an authContextKey
- With authorizeQueryContextParams enabled, this triggers a WRITE
permission check on QUERY_CONTEXT:remoteAddress
**After**:
- remoteAddress is removed from the session context map entirely
- Passed as a separate constructor/method parameter: DruidMeta →
DruidConnection → DruidJdbcStatement / PreparedStatement → SqlStatementFactory
→ DirectStatement
- No longer treated as a user-supplied context key, so no unintended auth
checks
This keeps it out of authContextKey entirely while still being available
for metrics/logging. Verified by building only the druid-sql jar and swapping
it into a running
cluster — works as expected.
<img width="1051" height="894" alt="image"
src="https://github.com/user-attachments/assets/1e051df6-dd22-48ab-8f9d-45c4562eb047";
/>
(test version: 37.0.0)
--
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]
