Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces credential hashing and caching optimizations to improve connection pooling and allow sensitive credentials to be garbage collected. Specifically, it adds SHA-256 hashing for OAuth tokens and encoded credentials in ConnectionOptions, refactors SpannerPool to use direct branching for credential resolution, and implements equals and hashCode in SpannerOmniCredentials. The reviewer identified critical issues in the SpannerOmniCredentials equality and hash code implementations: delegating to super.equals compares transient access tokens, which breaks connection pooling once a token is fetched, and including the mutable usePlainText field violates collection contracts and can corrupt the connection pool map. The reviewer recommended removing both from equals and hashCode and updating the associated unit tests accordingly.
8063fee to
8176e60
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces credential hashing and memory optimization for Spanner connection options, specifically hashing OAuth tokens and encoded credentials to allow the raw secrets to be garbage collected. It also refactors the credential key resolution in SpannerPool to use direct branching for better performance, and implements robust equals, hashCode, and target normalization in SpannerOmniCredentials. The review feedback highlights a potential NullPointerException in SpannerPool when resolving credentials from a non-null provider if the credentials themselves are null, and suggests adding a defensive null check.
8176e60 to
627843a
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request optimizes connection pooling and credential handling in the Cloud Spanner connection framework. Key changes include hashing sensitive credentials (OAuth tokens and encoded credentials) to allow the original values to be garbage collected, refactoring SpannerPool's CredentialsKey resolution to use direct branching instead of streams to reduce object allocation overhead, and implementing robust equals and hashCode methods for SpannerOmniCredentials with target normalization. Extensive unit tests have been added to verify these pooling behaviors and credential stability. I have no feedback to provide as there are no review comments.
Previously, Spanner Omni credentials and encoded service account credentials were not included when calculating connection pool keys. As a result, connections using different passwords, different Omni users, or different service account keys could reuse an existing authenticated client from the pool, potentially bypassing authentication or allowing cross-tenant client reuse. This change ensures that all distinct credentials receive isolated client instances in the connection pool: - Spanner Omni credentials now correctly evaluate username, target endpoint, transport security, and password equality (using constant-time comparisons), while excluding passwords from hash calculations to prevent offline cracking. - Encoded credentials and OAuth tokens are keyed in the pool using domain-prefixed SHA-256 hashes, ensuring client isolation while avoiding storing raw private keys or tokens in the long-lived pool cache. - Sensitive credential properties are cleared from connection startup state after client initialization so they can be promptly garbage collected.
627843a to
397fcd4
Compare
Previously, Spanner Omni credentials and encoded service account credentials were not included when calculating connection pool keys. As a result, connections using different passwords, different Omni users, or different service account keys could reuse an existing authenticated client from the pool, potentially bypassing authentication or allowing cross-tenant client reuse.
This change ensures that all distinct credentials receive isolated client instances in the connection pool: