wombatu-kun opened a new pull request, #16524:
URL: https://github.com/apache/iceberg/pull/16524

   Closes #16460.
   
   ## Summary
   
   `S3V4RestSignerClient` kept its `AuthManager` and `RESTClient` in 
process-wide `static volatile` fields, lazily initialized from the first 
catalog's properties. In a JVM that talks to more than one catalog the first 
catalog to initialize these fields won: any later catalog with a different auth 
type, different OAuth credentials, or different custom HTTP headers silently 
reused the first catalog's auth manager and HTTP client. That is the 
cross-catalog leakage reported in #16460. PR #14178 previously removed the 
base-URI pinning from the shared client but deliberately left the static fields 
in place; this change finishes the cleanup by making the auth/HTTP state per 
signer instance.
   
   ## What changed
   
   The `authManager` and `httpClient` fields are now per-instance 
(double-checked locking on the instance rather than the class), so each signer 
owns its own auth manager and HTTP client. One signer is created per `S3Client` 
build — bounded by the number of catalogs and storage prefixes, never per 
request — so this is cheap, and the previous "first catalog wins" sharing 
bought nothing.
   
   The HTTP client is now built through a settable `httpClientSupplier()` 
default, which keeps per-catalog headers and timeouts isolated and provides a 
seam for injecting a mock client in tests. The previously empty `close()` now 
releases the per-instance auth manager and HTTP client via 
`IoUtils.closeQuietlyV2`.
   
   The static `SIGNED_COMPONENT_CACHE` is intentionally left shared: it is 
keyed by request identity (method, region, URI) — a property of the S3 request 
being signed, not of any catalog or credential — so sharing it across catalogs 
is safe.
   
   ## Tests
   
   Added regression tests in `TestS3V4RestSignerClient` showing two signer 
instances with different configuration no longer share state: distinct HTTP 
clients per instance, distinct auth managers of distinct types when auth 
differs, and that `close()` releases the HTTP client. Reworked the existing 
unit and integration tests to inject the HTTP client per instance and to 
release the signer through `close()` instead of nulling the removed static 
fields.
   


-- 
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