fix: possible NPE when HttpStorageOptions deserialized (#2153)

BenWhitehead · web-flow · commit 68ad8e735709 · 2023-08-03T16:08:20.000-04:00
Java can dedupe objects in a cyclic traversal, retryingDepsAdapter doesn't need to be transient.

b/294399427
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java
@@ -52,7 +52,7 @@ public class HttpStorageOptions extends StorageOptions {
   private static final String DEFAULT_HOST = "https://storage.googleapis.com";
 
   private final HttpRetryAlgorithmManager retryAlgorithmManager;
-  private final transient RetryDependenciesAdapter retryDepsAdapter;
+  private final RetryDependenciesAdapter retryDepsAdapter;
 
   private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) {
     super(builder, serviceDefaults);
@@ -334,7 +334,7 @@ public ServiceRpc create(StorageOptions options) {
    * We don't yet want to make HttpStorageOptions itself implement {@link RetryingDependencies} but
    * we do need use it in a couple places, for those we create this adapter.
    */
-  private final class RetryDependenciesAdapter implements RetryingDependencies {
+  private final class RetryDependenciesAdapter implements RetryingDependencies, Serializable {
 
     private RetryDependenciesAdapter() {}
 
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/SerializationTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/SerializationTest.java
@@ -40,9 +40,11 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.util.Base64;
 import java.util.Collections;
@@ -213,6 +215,29 @@ protected Serializable[] serializableObjects() {
     }
   }
 
+  @Test
+  public void avoidNpeHttpStorageOptions_retryDeps() throws IOException, ClassNotFoundException {
+    HttpStorageOptions optionsHttp1 =
+        StorageOptions.http()
+            .setProjectId("http1")
+            .setCredentials(NoCredentials.getInstance())
+            .build();
+
+    assertThat(optionsHttp1.asRetryDependencies()).isNotNull();
+
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    try (ObjectOutputStream oos = new ObjectOutputStream(baos)) {
+      oos.writeObject(optionsHttp1);
+    }
+
+    byte[] byteArray = baos.toByteArray();
+    try (ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(byteArray))) {
+      Object o = ois.readObject();
+      HttpStorageOptions hso = (HttpStorageOptions) o;
+      assertThat(hso.asRetryDependencies()).isNotNull();
+    }
+  }
+
   @Override
   @SuppressWarnings("resource")
   protected Restorable<?>[] restorableObjects() {
