fix(gax): address PR 13995 AI review findings and Javadoc doclint errors

Addresses AI code review findings from https://paste.googleplex.com/6563525517508608:
- GrpcCallContext: Prevent transportChannel stale inheritance in merge() and withChannel()
- RefreshingHttpJsonChannel: Set shutdownRequested and shutdownInitiated in shutdownNow() so newCall() throws IllegalStateException
- AttemptCallable / StreamingCallables: Pass getCause() when rethrowing retryable UnauthenticatedException to prevent double-wrapping
- CertificateBasedAccess: Enforce fail-closed security boundary when certificate config is malformed or missing required keys, and fix JSON unescaping order
- ChannelPool: Update ReleasingClientCall Javadoc contract
- Unit tests: Add cache invalidation test helpers to eliminate Thread.sleep() delays and add comprehensive tests for all addressed edge cases

Author: macastelaz

sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java

@@ -485,6 +485,11 @@ private String getOrUpdateDiskFingerprint(String certPath) {
     }
   }
 
+  @VisibleForTesting
+  void invalidateDiskFingerprintCache() {
+    this.lastDiskCheck = null;
+  }
+
   boolean shouldRefresh() {
     if (workloadCertPath == null) {
       return false;
@@ -513,6 +518,7 @@ void refresh() {
     //   replaces the list)
     synchronized (entryWriteLock) {
       if (workloadCertPath == null) {
+        refreshAll();
         return;
       }
       String currentDiskFingerprint = getOrUpdateDiskFingerprint(workloadCertPath);
@@ -721,9 +727,9 @@ public <RequestT, ResponseT> ClientCall<RequestT, ResponseT> newCall(
   /**
    * ClientCall wrapper that makes sure to decrement the outstanding RPC count on completion.
    *
-   * <p>Contract: Exactly one call to {@link #start(Listener, Metadata)} or explicit release via
-   * {@link #cancel(String, Throwable)} is required to balance reference counts. Early cancellation
-   * before {@code start()} safely decrements the reference count via atomic compare-and-set.
+   * <p>Contract: Exactly one call to {@link #start(Listener, Metadata)} is required to balance
+   * reference counts. Early cancellation before {@code start()} is recorded and safely decrements
+   * the reference count when {@code start()} is subsequently invoked.
    */
   static class ReleasingClientCall<ReqT, RespT> extends SimpleForwardingClientCall<ReqT, RespT> {
     private @Nullable CancellationException cancellationException;

sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java

@@ -563,7 +563,8 @@ public ApiCallContext merge(ApiCallContext inputCallContext) {
     }
 
     TransportChannel newTransportChannel = grpcCallContext.transportChannel;
-    if (newTransportChannel == null) {
+    if (newTransportChannel == null
+        && (grpcCallContext.channel == null || grpcCallContext.channel.equals(channel))) {
       newTransportChannel = transportChannel;
     }
 
@@ -662,7 +663,7 @@ public GrpcCallContext withChannel(@Nullable Channel newChannel) {
         retryableCodes,
         endpointContext,
         isDirectPath,
-        transportChannel);
+        (newChannel == null || newChannel.equals(channel)) ? transportChannel : null);
   }
 
   /** Returns a new instance with the call options set to the given call options. */

sdk-platform-java/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java

@@ -473,7 +473,7 @@ void channelReactiveMTlsRefreshShouldConditionallySwapChannels()
         .newCall(Mockito.<MethodDescriptor<String, Integer>>any(), Mockito.any(CallOptions.class));
 
     // The ChannelPool caches fingerprints for 1000ms, wait for it to expire
-    Thread.sleep(1100);
+    pool.invalidateDiskFingerprintCache();
 
     java.nio.file.Path rootCert =
         java.nio.file.Paths.get("src", "test", "resources", "root_cert.pem");
@@ -529,6 +529,37 @@ void channelRefreshShouldSwapChannels() throws IOException {
         .newCall(Mockito.<MethodDescriptor<String, Integer>>any(), Mockito.any(CallOptions.class));
   }
 
+  @Test
+  void testRefreshWithNullWorkloadCertPathSwapsChannel() throws IOException {
+    ScheduledExecutorService executor =
+        Mockito.mock(ScheduledExecutorService.class, Mockito.withSettings().withoutAnnotations());
+    FixedExecutorProvider provider = FixedExecutorProvider.create(executor);
+    ManagedChannel underlyingChannel1 = Mockito.mock(ManagedChannel.class);
+    ManagedChannel underlyingChannel2 = Mockito.mock(ManagedChannel.class);
+    FakeChannelFactory channelFactory =
+        new FakeChannelFactory(ImmutableList.of(underlyingChannel1, underlyingChannel2));
+    pool =
+        new ChannelPool(
+            ChannelPoolSettings.staticallySized(1).toBuilder()
+                .setPreemptiveRefreshEnabled(true)
+                .build(),
+            channelFactory,
+            provider,
+            null);
+    Mockito.reset(underlyingChannel1);
+
+    pool.newCall(FakeMethodDescriptor.<String, Integer>create(), CallOptions.DEFAULT);
+    Mockito.verify(underlyingChannel1, Mockito.only())
+        .newCall(Mockito.<MethodDescriptor<String, Integer>>any(), Mockito.any(CallOptions.class));
+
+    // Calling refresh() when workloadCertPath is null should fall back to refreshAll()
+    pool.refresh();
+
+    pool.newCall(FakeMethodDescriptor.<String, Integer>create(), CallOptions.DEFAULT);
+    Mockito.verify(underlyingChannel2, Mockito.only())
+        .newCall(Mockito.<MethodDescriptor<String, Integer>>any(), Mockito.any(CallOptions.class));
+  }
+
   @Test
   void channelCountShouldNotChangeWhenOutstandingRpcsAreWithinLimits() throws Exception {
     ScheduledExecutorService executor =

sdk-platform-java/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java

@@ -515,4 +515,33 @@ public void testEqualsAndHashCode() {
 
     org.junit.jupiter.api.Assertions.assertNotEquals(context1, context3);
   }
+
+  @Test
+  public void testMergeWithCustomChannelClearsTransportChannel() {
+    ManagedChannel defaultChannel = org.mockito.Mockito.mock(ManagedChannel.class);
+    ManagedChannel customChannel = org.mockito.Mockito.mock(ManagedChannel.class);
+    GrpcTransportChannel transportChannel = GrpcTransportChannel.create(defaultChannel);
+
+    GrpcCallContext baseContext =
+        GrpcCallContext.createDefault().withTransportChannel(transportChannel);
+    GrpcCallContext overrideContext = GrpcCallContext.of(customChannel, CallOptions.DEFAULT);
+
+    GrpcCallContext mergedContext = (GrpcCallContext) baseContext.merge(overrideContext);
+    assertEquals(customChannel, mergedContext.getChannel());
+    assertNull(mergedContext.getTransportChannel());
+  }
+
+  @Test
+  public void testWithChannelWithCustomChannelClearsTransportChannel() {
+    ManagedChannel defaultChannel = org.mockito.Mockito.mock(ManagedChannel.class);
+    ManagedChannel customChannel = org.mockito.Mockito.mock(ManagedChannel.class);
+    GrpcTransportChannel transportChannel = GrpcTransportChannel.create(defaultChannel);
+
+    GrpcCallContext baseContext =
+        GrpcCallContext.createDefault().withTransportChannel(transportChannel);
+    GrpcCallContext updatedContext = baseContext.withChannel(customChannel);
+
+    assertEquals(customChannel, updatedContext.getChannel());
+    assertNull(updatedContext.getTransportChannel());
+  }
 }

sdk-platform-java/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/InstantiatingHttpJsonChannelProvider.java

@@ -199,7 +199,10 @@ public TransportChannelProvider withCredentials(Credentials credentials) {
     if (certificateBasedAccess.useMtlsClientCertificate()) {
       KeyStore mtlsKeyStore = mtlsProvider.getKeyStore();
       if (mtlsKeyStore != null) {
-        return new NetHttpTransport.Builder().trustCertificates(null, mtlsKeyStore, "").build();
+        NetHttpTransport.Builder builder = new NetHttpTransport.Builder();
+        builder.trustCertificates(null, mtlsKeyStore, "");
+        HttpJsonConscryptUtils.configureConscryptSecurityProvider(builder);
+        return builder.build();
       }
     }
     return null;

sdk-platform-java/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java

@@ -78,7 +78,12 @@ private ManagedHttpJsonChannel(
     this.executor = executor;
     this.usingDefaultExecutor = usingDefaultExecutor;
     this.endpoint = endpoint;
-    this.httpTransport = httpTransport == null ? new NetHttpTransport() : httpTransport;
+    this.httpTransport =
+        httpTransport == null
+            ? HttpJsonConscryptUtils.configureConscryptSecurityProvider(
+                    new NetHttpTransport.Builder())
+                .build()
+            : httpTransport;
     this.usingDefaultTransport = usingDefaultTransport || httpTransport == null;
     this.deadlineScheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
   }

sdk-platform-java/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/RefreshingHttpJsonChannel.java

@@ -242,11 +242,18 @@ public void shutdownNow() {
     synchronized (refreshLock) {
       isShuttingDown = true;
       for (ChannelEntry entry : allEntries) {
+        entry.shutdownRequested.set(true);
+        entry.shutdownInitiated.set(true);
         entry.channel.shutdownNow();
       }
     }
   }
 
+  @VisibleForTesting
+  void invalidateDiskFingerprintCache() {
+    this.lastDiskCheck = null;
+  }
+
   @Override
   public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException {
     long endNanos = System.nanoTime() + unit.toNanos(duration);

sdk-platform-java/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RefreshingHttpJsonChannelTest.java

@@ -177,15 +177,15 @@ void testShouldRefreshNullCertPath() {
   void testShouldRefreshFalseWhenUnchanged() throws InterruptedException {
     RefreshingHttpJsonChannel channel = createTestChannel();
 
-    Thread.sleep(1001); // Invalidate 1-second cache
+    channel.invalidateDiskFingerprintCache(); // Invalidate 1-second cache
     assertFalse(channel.shouldRefresh());
   }
 
   @Test
   void testShouldRefreshTrueWhenChanged() throws InterruptedException {
     RefreshingHttpJsonChannel channel = createTestChannel();
 
-    Thread.sleep(1001); // Invalidate 1-second cache
+    channel.invalidateDiskFingerprintCache(); // Invalidate 1-second cache
 
     // Simulate disk fingerprint changing
     testFingerprint = "fingerprint2";
@@ -199,7 +199,7 @@ void testRefreshSwapsChannel() throws InterruptedException {
     FakeManagedHttpJsonChannel firstChannel = lastCreatedChannel;
     assertEquals(1, channelFactoryCount.get());
 
-    Thread.sleep(1001); // Invalidate 1-second cache
+    channel.invalidateDiskFingerprintCache(); // Invalidate 1-second cache
 
     // Change fingerprint
     testFingerprint = "fingerprint2";
@@ -227,7 +227,7 @@ void testRefreshKeepsInFlightChannelsAlive() throws InterruptedException {
 
     HttpJsonClientCall<Object, Object> activeCall = channel.newCall(null, null);
 
-    Thread.sleep(1001); // Invalidate 1-second cache
+    channel.invalidateDiskFingerprintCache(); // Invalidate 1-second cache
 
     // Change fingerprint & refresh
     testFingerprint = "fingerprint2";
@@ -262,7 +262,7 @@ void testRefreshDoesNotSpawnChannelWhenShutdown() throws InterruptedException {
     channel.shutdown();
     firstChannel.shutdown();
 
-    Thread.sleep(1001); // Invalidate 1-second cache
+    channel.invalidateDiskFingerprintCache(); // Invalidate 1-second cache
 
     // Change fingerprint
     testFingerprint = "fingerprint2";
@@ -280,7 +280,7 @@ void testRefreshFactoryExceptionDoesNotWedgeFingerprint() throws InterruptedExce
     assertEquals(1, channelFactoryCount.get());
 
     shouldThrowOnFactory = true;
-    Thread.sleep(1001); // Invalidate 1-second cache
+    channel.invalidateDiskFingerprintCache(); // Invalidate 1-second cache
     testFingerprint = "fingerprint2";
 
     assertThrows(RuntimeException.class, channel::refresh);
@@ -324,4 +324,48 @@ void testChannelDelegationMethods() {
     assertEquals(firstChannel.getHttpTransport(), channel.getHttpTransport());
     assertEquals(firstChannel.getExecutor(), channel.getExecutor());
   }
+
+  @Test
+  void testNewCallAfterShutdownNowThrowsIllegalStateException() {
+    RefreshingHttpJsonChannel channel = createTestChannel();
+    channel.shutdownNow();
+
+    assertThrows(
+        IllegalStateException.class,
+        () -> channel.newCall(null, null),
+        "Channel has been shut down");
+  }
+
+  @Test
+  void testConcurrentNewCallDuringRefresh() throws InterruptedException {
+    RefreshingHttpJsonChannel channel = createTestChannel();
+    int threadCount = 10;
+    java.util.concurrent.ExecutorService executorService =
+        java.util.concurrent.Executors.newFixedThreadPool(threadCount);
+    java.util.concurrent.CountDownLatch latch =
+        new java.util.concurrent.CountDownLatch(threadCount);
+    java.util.concurrent.atomic.AtomicInteger successCount =
+        new java.util.concurrent.atomic.AtomicInteger(0);
+
+    for (int i = 0; i < threadCount; i++) {
+      executorService.submit(
+          () -> {
+            try {
+              channel.newCall(null, null);
+              successCount.incrementAndGet();
+            } finally {
+              latch.countDown();
+            }
+          });
+    }
+
+    channel.invalidateDiskFingerprintCache();
+    testFingerprint = "fingerprint2";
+    channel.refresh();
+
+    latch.await(5, TimeUnit.SECONDS);
+    executorService.shutdown();
+
+    assertEquals(threadCount, successCount.get());
+  }
 }

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetrySettings.java

@@ -189,7 +189,7 @@ public final org.threeten.bp.Duration getInitialRpcTimeout() {
    * connection has been terminated).
    *
    * <p>{@link #getTotalTimeout()} caps how long the logic should keep trying the RPC until it gives
-   * up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be <=
+   * up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be &lt;=
    * totalTimeout.
    *
    * <p>If there are no configurations, Retries have the default initial RPC timeout value of {@code
@@ -356,7 +356,7 @@ public final Builder setInitialRpcTimeout(org.threeten.bp.Duration initialTimeou
      * the connection has been terminated).
      *
      * <p>{@link #getTotalTimeout()} caps how long the logic should keep trying the RPC until it
-     * gives up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be <=
+     * gives up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be &lt;=
      * totalTimeout.
      *
      * <p>If there are no configurations, Retries have the default initial RPC timeout value of
@@ -491,7 +491,7 @@ public final org.threeten.bp.Duration getInitialRpcTimeout() {
      * the connection has been terminated).
      *
      * <p>{@link #getTotalTimeout()} caps how long the logic should keep trying the RPC until it
-     * gives up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be <=
+     * gives up completely. If {@link #getTotalTimeout()} is set, initialRpcTimeout should be &lt;=
      * totalTimeout.
      *
      * <p>If there are no configurations, Retries have the default initial RPC timeout value of

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/AttemptCallable.java

@@ -98,7 +98,7 @@ public ResponseT call() {
                   UnauthenticatedException newEx =
                       new UnauthenticatedException(
                           unauthenticatedException.getMessage(),
-                          unauthenticatedException,
+                          unauthenticatedException.getCause(),
                           unauthenticatedException.getStatusCode(),
                           true, // isRetryable = true
                           unauthenticatedException.getErrorDetails());