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. 
   
   
   <img width="1093" height="924" alt="image" 
src="https://github.com/user-attachments/assets/11d3f20a-0bdc-4320-8dc3-5b984e9dbb20";
 />
   
   (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]

Reply via email to