fix: a resumable session without a Range header should be interpreted as 0 length (#2182)

BenWhitehead · web-flow · commit 53022011d83e · 2023-08-30T13:39:18.000-04:00
According to https://cloud.google.com/storage/docs/performing-resumable-uploads#status-check a 308 response that does not contain a Range header should interpret as GCS having received no data. Include x-goog-gcs-idempotency-token in Json Resumable upload debug context
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/JsonResumableSessionFailureScenario.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/JsonResumableSessionFailureScenario.java
@@ -65,8 +65,7 @@ enum JsonResumableSessionFailureScenario {
       BaseServiceException.UNKNOWN_CODE,
       "dataLoss",
       "Client side data loss detected. Bytes acked is more than client sent."),
-  SCENARIO_9(503, "backendNotConnected", "Ack less than bytes sent"),
-  QUERY_SCENARIO_1(503, "", "Missing Range header in response");
+  SCENARIO_9(503, "backendNotConnected", "Ack less than bytes sent");
 
   private static final String PREFIX_I = "\t|< ";
   private static final String PREFIX_O = "\t|> ";
@@ -79,6 +78,7 @@ enum JsonResumableSessionFailureScenario {
           .or(matches("Content-Type"))
           .or(matches("Range"))
           .or(startsWith("X-Goog-Stored-"))
+          .or(matches("X-Goog-GCS-Idempotency-Token"))
           .or(matches("X-GUploader-UploadID"));
 
   private static final Predicate<Map.Entry<String, ?>> includeHeader =
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/JsonResumableSessionQueryTask.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/JsonResumableSessionQueryTask.java
@@ -97,8 +97,11 @@ final class JsonResumableSessionQueryTask
           long endOffset = range.endOffset();
           return ResumableOperationResult.incremental(endOffset);
         } else {
-          throw JsonResumableSessionFailureScenario.QUERY_SCENARIO_1.toStorageException(
-              uploadId, response);
+          // According to
+          // https://cloud.google.com/storage/docs/performing-resumable-uploads#status-check a 308
+          // response that does not contain a Range header should be interpreted as GCS having
+          // received no data.
+          return ResumableOperationResult.incremental(0);
         }
       } else {
         HttpResponseException cause = new HttpResponseException(response);
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITJsonResumableSessionQueryTaskTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITJsonResumableSessionQueryTaskTest.java
@@ -141,10 +141,6 @@ public void incompleteSession() throws Exception {
     }
   }
 
-  /**
-   * This is a hard failure from the perspective of GCS as a range header is a required header to be
-   * included in the response to a query upload request.
-   */
   @Test
   public void incompleteSession_missingRangeHeader() throws Exception {
     HttpRequestHandler handler =
@@ -156,9 +152,9 @@ public void incompleteSession_missingRangeHeader() throws Exception {
       JsonResumableSessionQueryTask task =
           new JsonResumableSessionQueryTask(httpClientContext, uploadUrl);
 
-      StorageException se = assertThrows(StorageException.class, task::call);
-      assertThat(se.getCode()).isEqualTo(503);
-      assertThat(se).hasMessageThat().contains("Range");
+      ResumableOperationResult<@Nullable StorageObject> result = task.call();
+      assertThat(result.getPersistedSize()).isEqualTo(0);
+      assertThat(result.getObject()).isNull();
     }
   }
 
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/JsonResumableSessionFailureScenarioTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/JsonResumableSessionFailureScenarioTest.java
@@ -139,6 +139,25 @@ public void xGoogStoredHeadersIncludedIfPresent() throws IOException {
     assertThat(storageException).hasMessageThat().contains("|< x-goog-stored-something: blah");
   }
 
+  @Test
+  public void xGoogGcsIdempotencyTokenHeadersIncludedIfPresent() throws IOException {
+    HttpRequest req =
+        new MockHttpTransport()
+            .createRequestFactory()
+            .buildPutRequest(new GenericUrl("http://localhost:80980"), new EmptyContent());
+    req.getHeaders().setContentLength(0L);
+
+    HttpResponse resp = req.execute();
+    resp.getHeaders().set("X-Goog-Gcs-Idempotency-Token", "5").setContentLength(0L);
+
+    StorageException storageException =
+        JsonResumableSessionFailureScenario.SCENARIO_0.toStorageException(
+            "uploadId", resp, null, () -> null);
+
+    assertThat(storageException.getCode()).isEqualTo(0);
+    assertThat(storageException).hasMessageThat().contains("|< x-goog-gcs-idempotency-token: 5");
+  }
+
   private static final class Cause extends RuntimeException {
 
     private Cause() {
