From 98521f2cc8ef60f10f514bf18226b7a737f0aa5e Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Fri, 25 Aug 2023 14:06:58 -0400 Subject: [PATCH 1/2] fix: a resumable session without a Range header should be interpreted as 0 length 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 --- .../JsonResumableSessionFailureScenario.java | 4 ++-- .../JsonResumableSessionQueryTask.java | 5 +++-- .../ITJsonResumableSessionQueryTaskTest.java | 10 +++------- ...onResumableSessionFailureScenarioTest.java | 19 +++++++++++++++++++ 4 files changed, 27 insertions(+), 11 deletions(-) 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 index 2b6e8d569c..0b6249ca8c 100644 --- 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> 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 index b37d2396d3..3dba9ac82a 100644 --- 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 @@ -92,13 +92,14 @@ final class JsonResumableSessionQueryTask } } else if (JsonResumableSessionFailureScenario.isContinue(code)) { String range1 = response.getHeaders().getRange(); + // if (range1 != null) { ByteRangeSpec range = ByteRangeSpec.parse(range1); long endOffset = range.endOffset(); return ResumableOperationResult.incremental(endOffset); } else { - throw JsonResumableSessionFailureScenario.QUERY_SCENARIO_1.toStorageException( - uploadId, response); + // https://cloud.google.com/storage/docs/performing-resumable-uploads#status-check + 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 index 07a04ed61e..d11cc340ec 100644 --- 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 index 7f5c7c7ac7..f40cf27f47 100644 --- 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() { From bf0fbb8e9656f7eaa8df3a3e334586de0e479fe9 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Tue, 29 Aug 2023 17:36:54 -0400 Subject: [PATCH 2/2] chore: review fixes --- .../google/cloud/storage/JsonResumableSessionQueryTask.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 index 3dba9ac82a..5ce0de6fe3 100644 --- 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 @@ -92,13 +92,15 @@ final class JsonResumableSessionQueryTask } } else if (JsonResumableSessionFailureScenario.isContinue(code)) { String range1 = response.getHeaders().getRange(); - // if (range1 != null) { ByteRangeSpec range = ByteRangeSpec.parse(range1); long endOffset = range.endOffset(); return ResumableOperationResult.incremental(endOffset); } else { - // https://cloud.google.com/storage/docs/performing-resumable-uploads#status-check + // 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 {