Skip to content

Commit 69d7f1f

Browse files
committed
addressed Gemini comments
1 parent ef72c3b commit 69d7f1f

5 files changed

Lines changed: 28 additions & 57 deletions

File tree

java-spanner/google-cloud-spanner/pom.xml

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -537,16 +537,6 @@
537537
<version>2.91.0</version><!-- {x-version-update:proto-google-cloud-trace-v1:current} -->
538538
<scope>test</scope>
539539
</dependency>
540-
<dependency>
541-
<groupId>org.bouncycastle</groupId>
542-
<artifactId>bcprov-jdk18on</artifactId>
543-
<version>1.78</version>
544-
</dependency>
545-
<dependency>
546-
<groupId>com.google.crypto.tink</groupId>
547-
<artifactId>tink</artifactId>
548-
<version>1.13.0</version>
549-
</dependency>
550540
</dependencies>
551541
<profiles>
552542
<profile>
@@ -744,16 +734,6 @@
744734
<groupId>com.google.api.grpc</groupId>
745735
<artifactId>grpc-google-cloud-spanner-executor-v1</artifactId>
746736
</dependency>
747-
<dependency>
748-
<groupId>org.bouncycastle</groupId>
749-
<artifactId>bcprov-jdk18on</artifactId>
750-
<version>1.78</version>
751-
</dependency>
752-
<dependency>
753-
<groupId>com.google.crypto.tink</groupId>
754-
<artifactId>tink</artifactId>
755-
<version>1.13.0</version>
756-
</dependency>
757737
</dependencies>
758738
</profile>
759739
</profiles>

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,8 +1849,7 @@ public Builder login(String username, String passwordFile, boolean backgroundRef
18491849
while (len > 0 && (rawBytes[len - 1] == '\n' || rawBytes[len - 1] == '\r')) {
18501850
len--;
18511851
}
1852-
byte[] passwordBytes = new byte[len];
1853-
System.arraycopy(rawBytes, 0, passwordBytes, 0, len);
1852+
byte[] passwordBytes = java.util.Arrays.copyOf(rawBytes, len);
18541853
return loginWithPasswordBytes(username, passwordBytes, backgroundRefresh);
18551854
} catch (IOException e) {
18561855
throw SpannerExceptionFactory.newSpannerException(

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/LoginClient.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
* authenticate to Spanner Omni using username/password.
4040
*/
4141
public class LoginClient {
42+
private static final java.security.SecureRandom SECURE_RANDOM = new java.security.SecureRandom();
43+
4244
private final LoginServiceGrpc.LoginServiceStub stub;
4345

4446
public LoginClient(ManagedChannel channel) {
@@ -63,9 +65,8 @@ public AccessToken login(String username, byte[] password) throws SpannerExcepti
6365
byte[] clientPrivateKeyshare = keyPair[0];
6466
byte[] clientPublicKeyshare = keyPair[1];
6567
byte[] clientNonce = OpaqueUtil.nonce();
66-
java.security.SecureRandom random = new java.security.SecureRandom();
6768
byte[] blind = new byte[32];
68-
random.nextBytes(blind);
69+
SECURE_RANDOM.nextBytes(blind);
6970

7071
byte[] blindedMessage = OpaqueUtil.blind(password, blind);
7172

@@ -249,10 +250,7 @@ LoginResponse getResponse() throws InterruptedException {
249250
if (error != null) {
250251
throw SpannerExceptionFactory.newSpannerException(error);
251252
}
252-
if (response == LoginResponse.getDefaultInstance() && (completed || error != null)) {
253-
if (error != null) {
254-
throw SpannerExceptionFactory.newSpannerException(error);
255-
}
253+
if (response == LoginResponse.getDefaultInstance() && completed) {
256254
return null;
257255
}
258256
return response;

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/SpannerOmniCredentials.java

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ public class SpannerOmniCredentials extends GoogleCredentials {
4848
private final byte[] password;
4949
private final String target;
5050
private final boolean backgroundRefresh;
51+
private final ManagedChannel loginChannel;
5152

5253
private ScheduledFuture<?> refreshTask;
5354

@@ -60,13 +61,13 @@ public SpannerOmniCredentials(String username, byte[] password, String target, b
6061
this.password = password;
6162
this.target = target;
6263
this.backgroundRefresh = backgroundRefresh;
64+
this.loginChannel = ManagedChannelBuilder.forTarget(target).build();
6365
}
6466

6567
@Override
6668
public AccessToken refreshAccessToken() throws IOException {
67-
ManagedChannel loginChannel = ManagedChannelBuilder.forTarget(target).build();
6869
try {
69-
LoginClient loginClient = new LoginClient(loginChannel);
70+
LoginClient loginClient = new LoginClient(this.loginChannel);
7071
google.spanner.omni.v1.AccessToken protoToken = loginClient.login(username, password);
7172
String tokenValue = Base64.getEncoder().encodeToString(protoToken.toByteArray());
7273

@@ -90,16 +91,6 @@ public AccessToken refreshAccessToken() throws IOException {
9091
} catch (Exception e) {
9192
logger.log(Level.SEVERE, "Failed to login to Spanner Omni. Username: " + username + ", Target: " + target, e);
9293
throw new IOException("Failed to login to Spanner Omni", e);
93-
} finally {
94-
loginChannel.shutdown();
95-
try {
96-
if (!loginChannel.awaitTermination(1, TimeUnit.SECONDS)) {
97-
loginChannel.shutdownNow();
98-
}
99-
} catch (InterruptedException ie) {
100-
Thread.currentThread().interrupt();
101-
loginChannel.shutdownNow();
102-
}
10394
}
10495
}
10596

@@ -123,21 +114,26 @@ private void scheduleRefresh(long tokenLifetimeMillis) {
123114

124115
java.lang.ref.WeakReference<SpannerOmniCredentials> weakThis = new java.lang.ref.WeakReference<>(this);
125116

126-
refreshTask = SHARED_EXECUTOR.schedule(() -> {
127-
SpannerOmniCredentials creds = weakThis.get();
128-
if (creds == null) {
129-
// The credentials instance was garbage collected. Stop the background refresh loop.
130-
return;
131-
}
132-
try {
133-
creds.refresh();
134-
} catch (IOException e) {
135-
logger.log(Level.WARNING, "Failed to auto-refresh Spanner Omni credentials", e);
136-
// Retry in a short interval on failure
137-
long retryDelay = Math.min(TimeUnit.SECONDS.toMillis(5), tokenLifetimeMillis / 4);
138-
if (retryDelay <= 0) retryDelay = 1000;
139-
creds.scheduleRefresh(retryDelay * 2); // Pass double the retry delay as fake lifetime to get retryDelay scheduling
117+
Runnable refreshAction = new Runnable() {
118+
@Override
119+
public void run() {
120+
SpannerOmniCredentials creds = weakThis.get();
121+
if (creds == null) {
122+
// The credentials instance was garbage collected. Stop the background refresh loop.
123+
return;
124+
}
125+
try {
126+
creds.refresh();
127+
} catch (IOException e) {
128+
logger.log(Level.WARNING, "Failed to auto-refresh Spanner Omni credentials", e);
129+
// Retry in a short interval on failure
130+
long retryDelay = Math.min(TimeUnit.SECONDS.toMillis(5), tokenLifetimeMillis / 4);
131+
if (retryDelay <= 0) retryDelay = 1000;
132+
creds.refreshTask = SHARED_EXECUTOR.schedule(this, retryDelay, TimeUnit.MILLISECONDS);
133+
}
140134
}
141-
}, delayMillis, TimeUnit.MILLISECONDS);
135+
};
136+
137+
refreshTask = SHARED_EXECUTOR.schedule(refreshAction, delayMillis, TimeUnit.MILLISECONDS);
142138
}
143139
}

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/omni/opaque/OpaqueUtil.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,6 @@ public static byte[] randomOracleSha256(byte[] x, BigInteger max) throws General
304304
byte[] tmp = new byte[iBytes.length - 1];
305305
System.arraycopy(iBytes, 1, tmp, 0, tmp.length);
306306
iBytes = tmp;
307-
} else if (iBytes.length == 0) {
308-
iBytes = new byte[]{0}; // Shouldn't happen for i >= 1
309307
}
310308

311309
byte[] bignumBytes;

0 commit comments

Comments
 (0)