Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 6c9c450

Browse files
committedMay 30, 2024
fix: update grpc bidi resumable uploads to validate ack'd object size
Follow up to #2527 Move request trimming inline rather than at the end
1 parent e2c3c81 commit 6c9c450

File tree

2 files changed

+16
-34
lines changed

2 files changed

+16
-34
lines changed
 

‎google-cloud-storage/src/main/java/com/google/cloud/storage/BidiResumableWrite.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public StartResumableWriteResponse getRes() {
5252

5353
@Override
5454
public BidiWriteObjectRequest.Builder newBuilder() {
55-
return writeRequest.toBuilder();
55+
return writeRequest.toBuilder().clearWriteObjectSpec();
5656
}
5757

5858
@Override

‎google-cloud-storage/src/main/java/com/google/cloud/storage/GapicBidiUnbufferedWritableByteChannel.java

+15-33
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ private long internalWrite(ByteBuffer[] srcs, int srcsOffset, int srcsLength, bo
136136
List<BidiWriteObjectRequest> messages = new ArrayList<>();
137137

138138
int bytesConsumed = 0;
139-
for (ChunkSegment datum : data) {
139+
for (int i = 0; i < data.length; i++) {
140+
ChunkSegment datum = data[i];
140141
Crc32cLengthKnown crc32c = datum.getCrc32c();
141142
ByteString b = datum.getB();
142143
int contentSize = b.size();
@@ -149,11 +150,14 @@ private long internalWrite(ByteBuffer[] srcs, int srcsOffset, int srcsLength, bo
149150
if (crc32c != null) {
150151
checksummedData.setCrc32C(crc32c.getValue());
151152
}
152-
BidiWriteObjectRequest.Builder builder =
153-
writeCtx
154-
.newRequestBuilder()
155-
.setWriteOffset(offset)
156-
.setChecksummedData(checksummedData.build());
153+
BidiWriteObjectRequest.Builder builder = writeCtx.newRequestBuilder();
154+
if (!first) {
155+
builder.clearUploadId();
156+
builder.clearObjectChecksums();
157+
} else {
158+
first = false;
159+
}
160+
builder.setWriteOffset(offset).setChecksummedData(checksummedData.build());
157161
if (!datum.isOnlyFullBlocks()) {
158162
builder.setFinishWrite(true);
159163
if (cumulative != null) {
@@ -163,8 +167,11 @@ private long internalWrite(ByteBuffer[] srcs, int srcsOffset, int srcsLength, bo
163167
finished = true;
164168
}
165169

166-
BidiWriteObjectRequest build = possiblyPairDownBidiRequest(builder, first).build();
167-
first = false;
170+
if (i == data.length - 1 && !finished) {
171+
builder.setFlush(true).setStateLookup(true);
172+
}
173+
174+
BidiWriteObjectRequest build = builder.build();
168175
messages.add(build);
169176
bytesConsumed += contentSize;
170177
}
@@ -224,11 +231,6 @@ private void flush(@NonNull List<BidiWriteObjectRequest> segments) {
224231
for (BidiWriteObjectRequest message : segments) {
225232
opened.onNext(message);
226233
}
227-
if (!finished) {
228-
BidiWriteObjectRequest message =
229-
BidiWriteObjectRequest.newBuilder().setFlush(true).setStateLookup(true).build();
230-
opened.onNext(message);
231-
}
232234
responseObserver.await();
233235
return null;
234236
} catch (Exception e) {
@@ -240,26 +242,6 @@ private void flush(@NonNull List<BidiWriteObjectRequest> segments) {
240242
Decoder.identity());
241243
}
242244

243-
private static BidiWriteObjectRequest.Builder possiblyPairDownBidiRequest(
244-
BidiWriteObjectRequest.Builder b, boolean firstMessageOfStream) {
245-
if (firstMessageOfStream && b.getWriteOffset() == 0) {
246-
return b;
247-
}
248-
249-
if (!firstMessageOfStream) {
250-
b.clearUploadId();
251-
}
252-
253-
if (b.getWriteOffset() > 0) {
254-
b.clearWriteObjectSpec();
255-
}
256-
257-
if (b.getWriteOffset() > 0 && !b.getFinishWrite()) {
258-
b.clearObjectChecksums();
259-
}
260-
return b;
261-
}
262-
263245
private class BidiObserver implements ApiStreamObserver<BidiWriteObjectResponse> {
264246

265247
private final Semaphore sem;

0 commit comments

Comments
 (0)