Skip to content

Commit 61824e6

Browse files
BenWhiteheadBenWhitehead
committed
chore: add request validation for BlobDescriptor
Validate and cap the length of a read to the object size. If a read is 0 bytes, don't actually send a request, instead return a future of the empty value.
1 parent 67482f7 commit 61824e6

File tree

9 files changed

+490
-45
lines changed

9 files changed

+490
-45
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public interface BlobDescriptor extends AutoCloseable, Closeable {
2626

2727
BlobInfo getBlobInfo();
2828

29-
ApiFuture<byte[]> readRangeAsBytes(ByteRangeSpec range);
29+
ApiFuture<byte[]> readRangeAsBytes(RangeSpec range);
3030

3131
@Override
3232
void close() throws IOException;

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

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,19 @@
2020
import com.google.api.core.ApiFutures;
2121
import com.google.api.core.SettableApiFuture;
2222
import com.google.api.gax.grpc.GrpcCallContext;
23+
import com.google.api.gax.grpc.GrpcStatusCode;
24+
import com.google.api.gax.rpc.ApiExceptionFactory;
2325
import com.google.cloud.storage.BlobDescriptor.ZeroCopySupport.DisposableByteString;
2426
import com.google.cloud.storage.BlobDescriptorStreamRead.AccumulatingRead;
2527
import com.google.cloud.storage.GrpcUtils.ZeroCopyBidiStreamingCallable;
28+
import com.google.common.annotations.VisibleForTesting;
29+
import com.google.common.math.LongMath;
30+
import com.google.protobuf.ByteString;
2631
import com.google.storage.v2.BidiReadObjectRequest;
2732
import com.google.storage.v2.BidiReadObjectResponse;
33+
import io.grpc.Status.Code;
2834
import java.io.IOException;
35+
import java.util.OptionalLong;
2936
import java.util.concurrent.Executor;
3037

3138
final class BlobDescriptorImpl implements BlobDescriptor {
@@ -41,23 +48,32 @@ private BlobDescriptorImpl(BlobDescriptorStream stream, BlobDescriptorState stat
4148
}
4249

4350
@Override
44-
public ApiFuture<byte[]> readRangeAsBytes(ByteRangeSpec range) {
51+
public ApiFuture<byte[]> readRangeAsBytes(RangeSpec range) {
4552
long readId = state.newReadId();
53+
ReadCursor readCursor = getReadCursor(range, state);
54+
if (!readCursor.hasRemaining()) {
55+
return ApiFutures.immediateFuture(new byte[0]);
56+
}
4657
SettableApiFuture<byte[]> future = SettableApiFuture.create();
4758
AccumulatingRead<byte[]> read =
48-
BlobDescriptorStreamRead.createByteArrayAccumulatingRead(readId, range, future);
59+
BlobDescriptorStreamRead.createByteArrayAccumulatingRead(readId, readCursor, future);
4960
BidiReadObjectRequest request =
5061
BidiReadObjectRequest.newBuilder().addReadRanges(read.makeReadRange()).build();
5162
state.putOutstandingRead(readId, read);
5263
stream.send(request);
5364
return future;
5465
}
5566

56-
public ApiFuture<DisposableByteString> readRangeAsByteString(ByteRangeSpec range) {
67+
public ApiFuture<DisposableByteString> readRangeAsByteString(RangeSpec range) {
5768
long readId = state.newReadId();
69+
ReadCursor readCursor = getReadCursor(range, state);
70+
if (!readCursor.hasRemaining()) {
71+
return ApiFutures.immediateFuture(EmptyDisposableByteString.INSTANCE);
72+
}
5873
SettableApiFuture<DisposableByteString> future = SettableApiFuture.create();
5974
AccumulatingRead<DisposableByteString> read =
60-
BlobDescriptorStreamRead.createZeroCopyByteStringAccumulatingRead(readId, range, future);
75+
BlobDescriptorStreamRead.createZeroCopyByteStringAccumulatingRead(
76+
readId, readCursor, future);
6177
BidiReadObjectRequest request =
6278
BidiReadObjectRequest.newBuilder().addReadRanges(read.makeReadRange()).build();
6379
state.putOutstandingRead(readId, read);
@@ -75,6 +91,26 @@ public void close() throws IOException {
7591
stream.close();
7692
}
7793

94+
@VisibleForTesting
95+
static ReadCursor getReadCursor(RangeSpec range, BlobDescriptorState state) {
96+
long begin = range.begin();
97+
long objectSize = state.getMetadata().getSize();
98+
if (begin > objectSize) {
99+
throw ApiExceptionFactory.createException(
100+
String.format(
101+
"range begin must be < objectSize (range begin = %d, object size = %d",
102+
begin, objectSize),
103+
null,
104+
GrpcStatusCode.of(Code.OUT_OF_RANGE),
105+
false);
106+
}
107+
final long end;
108+
OptionalLong limit = range.limit();
109+
long saturatedAdd = LongMath.saturatedAdd(begin, limit.orElse(0L));
110+
end = Math.min(saturatedAdd, objectSize);
111+
return new ReadCursor(begin, end);
112+
}
113+
78114
static ApiFuture<BlobDescriptor> create(
79115
BidiReadObjectRequest openRequest,
80116
GrpcCallContext context,
@@ -89,4 +125,20 @@ static ApiFuture<BlobDescriptor> create(
89125
stream.send(openRequest);
90126
return StorageException.coalesceAsync(blobDescriptorFuture);
91127
}
128+
129+
private static final class EmptyDisposableByteString implements DisposableByteString {
130+
private static final EmptyDisposableByteString INSTANCE = new EmptyDisposableByteString();
131+
132+
private EmptyDisposableByteString() {}
133+
134+
@Override
135+
public ByteString byteString() {
136+
return ByteString.empty();
137+
}
138+
139+
@Override
140+
public void close() throws IOException {
141+
// no-op
142+
}
143+
}
92144
}

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,19 +82,13 @@ public void close() throws IOException {
8282
}
8383

8484
static AccumulatingRead<byte[]> createByteArrayAccumulatingRead(
85-
long readId, ByteRangeSpec range, SettableApiFuture<byte[]> complete) {
86-
return new ByteArrayAccumulatingRead(
87-
readId,
88-
new ReadCursor(range.beginOffset(), range.beginOffset() + range.length()),
89-
complete);
85+
long readId, ReadCursor readCursor, SettableApiFuture<byte[]> complete) {
86+
return new ByteArrayAccumulatingRead(readId, readCursor, complete);
9087
}
9188

92-
static AccumulatingRead<DisposableByteString> createZeroCopyByteStringAccumulatingRead(
93-
long readId, ByteRangeSpec range, SettableApiFuture<DisposableByteString> complete) {
94-
return new ZeroCopyByteStringAccumulatingRead(
95-
readId,
96-
new ReadCursor(range.beginOffset(), range.beginOffset() + range.length()),
97-
complete);
89+
static ZeroCopyByteStringAccumulatingRead createZeroCopyByteStringAccumulatingRead(
90+
long readId, ReadCursor readCursor, SettableApiFuture<DisposableByteString> complete) {
91+
return new ZeroCopyByteStringAccumulatingRead(readId, readCursor, complete);
9892
}
9993

10094
/** Base class of a read that will accumulate before completing by resolving a future */

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
*/
4444
@InternalApi
4545
@ThreadSafe
46-
public abstract class ByteRangeSpec implements Serializable {
46+
abstract class ByteRangeSpec implements Serializable {
4747

4848
public static final long EFFECTIVE_INFINITY = Long.MAX_VALUE;
4949

@@ -111,20 +111,19 @@ public String toString() {
111111

112112
protected abstract MoreObjects.ToStringHelper append(MoreObjects.ToStringHelper tsh);
113113

114-
public static ByteRangeSpec nullRange() {
114+
static ByteRangeSpec nullRange() {
115115
return NullByteRangeSpec.INSTANCE;
116116
}
117117

118-
public static ByteRangeSpec relativeLength(@Nullable Long beginOffset, @Nullable Long length) {
118+
static ByteRangeSpec relativeLength(@Nullable Long beginOffset, @Nullable Long length) {
119119
return create(beginOffset, length, RelativeByteRangeSpec::new);
120120
}
121121

122-
public static ByteRangeSpec explicit(
123-
@Nullable Long beginOffset, @Nullable Long endOffsetExclusive) {
122+
static ByteRangeSpec explicit(@Nullable Long beginOffset, @Nullable Long endOffsetExclusive) {
124123
return create(beginOffset, endOffsetExclusive, LeftClosedRightOpenByteRangeSpec::new);
125124
}
126125

127-
public static ByteRangeSpec explicitClosed(
126+
static ByteRangeSpec explicitClosed(
128127
@Nullable Long beginOffset, @Nullable Long endOffsetInclusive) {
129128
return create(beginOffset, endOffsetInclusive, LeftClosedRightClosedByteRangeSpec::new);
130129
}

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

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
/*
2+
* Copyright 2024 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.storage;
18+
19+
import static com.google.common.base.Preconditions.checkArgument;
20+
21+
import com.google.common.base.MoreObjects;
22+
import java.util.Objects;
23+
import java.util.OptionalLong;
24+
import javax.annotation.concurrent.Immutable;
25+
import org.checkerframework.checker.nullness.qual.NonNull;
26+
27+
/** Defines a range with begin offset and limit. */
28+
@Immutable
29+
public abstract class RangeSpec {
30+
// seal this class to extension
31+
private RangeSpec() {}
32+
33+
/** The beginning of the range. */
34+
public abstract long begin();
35+
36+
/**
37+
* A limit of the range if defined.
38+
*
39+
* @see RangeSpecWithLimit
40+
*/
41+
public abstract OptionalLong limit();
42+
43+
/**
44+
* Create a new instance of {@link RangeSpec} keeping {@code this.begin()} and with {@code limit}
45+
* as its new limit.
46+
*/
47+
@NonNull
48+
public abstract RangeSpec withLimit(long limit);
49+
50+
/** {@inheritDoc} */
51+
@Override
52+
public abstract boolean equals(Object o);
53+
54+
/** {@inheritDoc} */
55+
@Override
56+
public abstract int hashCode();
57+
58+
/** {@inheritDoc} */
59+
@Override
60+
public abstract String toString();
61+
62+
/**
63+
* Create a new RangeSpec with the provided {@code begin}.
64+
*
65+
* @throws IllegalArgumentException if begin is &lt; 0
66+
*/
67+
@NonNull
68+
public static RangeSpec beginAt(long begin) {
69+
checkArgument(begin >= 0, "range being must be >= 0 (range begin = %s)", begin);
70+
return new RangeSpecWithoutLimit(begin);
71+
}
72+
73+
/**
74+
* Create a new RangeSpec with the provided {@code begin} and {@code limit}.
75+
*
76+
* @throws IllegalArgumentException if begin is &lt; 0, or if limit is &lt; 0
77+
*/
78+
@NonNull
79+
public static RangeSpec of(long begin, long limit) {
80+
checkArgument(begin >= 0, "range being must be >= 0 (range begin = %s)", begin);
81+
checkArgument(limit >= 0, "range limit must be >= 0 (range limit = %s)", limit);
82+
return new RangeSpecWithLimit(begin, limit);
83+
}
84+
85+
/** A RangeSpec that represents to read from {@code 0} to {@code EOF} */
86+
@NonNull
87+
public static RangeSpec all() {
88+
return RangeSpecWithoutLimit.ALL;
89+
}
90+
91+
static final class RangeSpecWithoutLimit extends RangeSpec {
92+
private static final RangeSpecWithoutLimit ALL = new RangeSpecWithoutLimit(0);
93+
private final long begin;
94+
95+
private RangeSpecWithoutLimit(long begin) {
96+
this.begin = begin;
97+
}
98+
99+
@Override
100+
public long begin() {
101+
return begin;
102+
}
103+
104+
@Override
105+
public OptionalLong limit() {
106+
return OptionalLong.empty();
107+
}
108+
109+
@Override
110+
@NonNull
111+
public RangeSpec withLimit(long limit) {
112+
checkArgument(limit >= 0, "range limit must be >= 0 (range limit = %s)", limit);
113+
return new RangeSpecWithLimit(begin, limit);
114+
}
115+
116+
@Override
117+
public boolean equals(Object o) {
118+
if (this == o) {
119+
return true;
120+
}
121+
if (!(o instanceof RangeSpecWithoutLimit)) {
122+
return false;
123+
}
124+
RangeSpecWithoutLimit that = (RangeSpecWithoutLimit) o;
125+
return begin == that.begin;
126+
}
127+
128+
@Override
129+
public int hashCode() {
130+
return Objects.hashCode(begin);
131+
}
132+
133+
@Override
134+
public String toString() {
135+
return MoreObjects.toStringHelper(this).add("begin", begin).toString();
136+
}
137+
}
138+
139+
static final class RangeSpecWithLimit extends RangeSpec {
140+
private final long begin;
141+
private final long limit;
142+
143+
private RangeSpecWithLimit(long begin, long limit) {
144+
this.begin = begin;
145+
this.limit = limit;
146+
}
147+
148+
@Override
149+
public long begin() {
150+
return begin;
151+
}
152+
153+
@Override
154+
public OptionalLong limit() {
155+
return OptionalLong.of(limit);
156+
}
157+
158+
@Override
159+
@NonNull
160+
public RangeSpec withLimit(long limit) {
161+
checkArgument(limit >= 0, "range limit must be >= 0 (range limit = %s)", limit);
162+
return new RangeSpecWithLimit(begin, limit);
163+
}
164+
165+
@Override
166+
public boolean equals(Object o) {
167+
if (this == o) {
168+
return true;
169+
}
170+
if (!(o instanceof RangeSpecWithLimit)) {
171+
return false;
172+
}
173+
RangeSpecWithLimit that = (RangeSpecWithLimit) o;
174+
return begin == that.begin && limit == that.limit;
175+
}
176+
177+
@Override
178+
public int hashCode() {
179+
return Objects.hash(begin, limit);
180+
}
181+
182+
@Override
183+
public String toString() {
184+
return MoreObjects.toStringHelper(RangeSpec.class)
185+
.add("begin", begin)
186+
.add("limit", limit)
187+
.toString();
188+
}
189+
}
190+
}

0 commit comments

Comments
 (0)