Skip to content
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2db4acb
feat(storage): Adding CumulativeHasher wrapper class for Full object …
May 20, 2026
351729c
Fixing lint and small updates
May 25, 2026
72af919
feat(storage): add full object checksum validation for grpc flow
May 25, 2026
91e8f36
fix: fix lint
May 25, 2026
e67803a
feat(storage): add full object checksum validation for bidi flow
May 25, 2026
a24e32a
Disabling cumulative hasher for noop
May 26, 2026
5c3c71a
Merge branch 'main' into add-cumulative-hasher-bidi-flow
Dhriti07 Jun 16, 2026
dc264f8
Merge branch 'main' into add-cumulative-hasher-bidi-flow
Dhriti07 Jun 17, 2026
6a02a24
adding disabling for bidi reads
Jun 17, 2026
4b3c67e
format fixes
Jun 17, 2026
1c1e08e
Merge branch 'add-cumulative-hasher-bidi-flow' into disable-bidi-chec…
Dhriti07 Jun 17, 2026
14c148b
Merge branch 'main' into disable-bidi-checksum-new
Dhriti07 Jun 18, 2026
31f5144
fromatting fixes
Jun 18, 2026
4e03119
Removing noop check from grpc path
Jun 18, 2026
13e89cb
fix previous commit
Jun 18, 2026
78f32bd
fix lint
Jun 18, 2026
d1723f2
Add system property for read hasher disabling
Jun 18, 2026
9d44e29
Merge branch 'main' into disable-bidi-checksum-new
nidhiii-27 Jun 19, 2026
a44d597
formatting fixes
Jun 22, 2026
04396b2
Merge branch 'main' into disable-bidi-checksum-new
Dhriti07 Jun 22, 2026
36140bf
feat(storage): respect com.google.cloud.storage.Hasher.read system pr…
nidhiii-27 Jun 24, 2026
409d83a
Merge branch 'main' into disable-bidi-checksum-new
Dhriti07 Jun 24, 2026
99082c9
Reverting noop change
Jun 24, 2026
fa820aa
Merge branch 'main' into disable-bidi-checksum-new
Dhriti07 Jun 24, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ private AccumulatingRead(
super(rangeSpec, retryContext, onCloseCallback);
this.readId = readId;
this.hasher =
(rangeSpec.begin() == 0 && !(hasher instanceof Hasher.NoOpHasher))
(rangeSpec.begin() == 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

By removing the !(hasher instanceof Hasher.NoOpHasher) check, a CumulativeHasher is now always allocated and wrapped around the delegate hasher when rangeSpec.begin() == 0, even if checksum validation is disabled (i.e., when hasher is a NoOpHasher). Keeping this check avoids unnecessary object allocation and overhead when checksumming is disabled.

Suggested change
(rangeSpec.begin() == 0)
(rangeSpec.begin() == 0 && !(hasher instanceof Hasher.NoOpHasher))
References
  1. Prefer lazy initialization over eager initialization for resource-intensive objects if they are not guaranteed to be used in all execution paths, to avoid unnecessary performance and memory overhead.

? new CumulativeHasher(hasher, 0, rangeSpec.maxLength())
: hasher;
this.complete = SettableApiFuture.create();
Expand Down Expand Up @@ -284,7 +284,7 @@ static class StreamingRead extends BaseObjectReadSessionStreamRead<ScatteringByt
super(rangeSpec, retryContext, onCloseCallback);
this.readId = new AtomicLong(readId);
this.hasher =
(rangeSpec.begin() == 0 && !(hasher instanceof Hasher.NoOpHasher))
(rangeSpec.begin() == 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

By removing the !(hasher instanceof Hasher.NoOpHasher) check, a CumulativeHasher is now always allocated and wrapped around the delegate hasher when rangeSpec.begin() == 0, even if checksum validation is disabled (i.e., when hasher is a NoOpHasher). Keeping this check avoids unnecessary object allocation and overhead when checksumming is disabled.

Suggested change
(rangeSpec.begin() == 0)
(rangeSpec.begin() == 0 && !(hasher instanceof Hasher.NoOpHasher))
References
  1. Prefer lazy initialization over eager initialization for resource-intensive objects if they are not guaranteed to be used in all execution paths, to avoid unnecessary performance and memory overhead.

? new CumulativeHasher(hasher, 0, rangeSpec.maxLength())
: hasher;
this.closed = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ final class CumulativeHasher implements Hasher {
this.delegate = delegate;
this.startOffset = startOffset;
this.limit = limit;
this.cumulativeHash = Crc32cValue.zero();
this.cumulativeHash = delegate.initialValue();
}

@Override
Expand Down Expand Up @@ -123,16 +123,19 @@ boolean qualifiesForVerification(Object metadata) {
void validateCumulativeChecksum(Object metadata)
throws UncheckedCumulativeChecksumMismatchException {
if (qualifiesForVerification(metadata)) {
Crc32cValue<?> expected = Crc32cValue.of(metadata.getChecksums().getCrc32C());
Crc32cLengthKnown actual = getCumulativeHash();
if (actual == null) {
return;
}
Crc32cValue<?> expected = Crc32cValue.of(metadata.getChecksums().getCrc32C());
if (!actual.eqValue(expected)) {
throw new UncheckedCumulativeChecksumMismatchException(expected, actual);
}
}
}

private void accumulate(Crc32cLengthKnown actual) {
cumulativeHash = cumulativeHash.concat(actual);
cumulativeHash = nullSafeConcat(cumulativeHash, actual);
}

Crc32cLengthKnown getCumulativeHash() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private ReadableByteChannelSessionBuilder(
this.read = read;
this.retrier = retrier;
this.resultRetryAlgorithm = resultRetryAlgorithm;
this.hasher = Hasher.defaultHasher();
this.hasher = Hasher.readHasher();
this.autoGzipDecompression = false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ final class GapicUnbufferedReadableByteChannel
this.read = read;
this.req = req;
this.hasher =
(req.getReadOffset() == 0 && !(hasher instanceof Hasher.NoOpHasher))
(req.getReadOffset() == 0)
? new CumulativeHasher(
hasher,
0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ protected LazyReadChannel<?, Object> newLazyReadChannel() {
ResumableMedia.gapic()
.read()
.byteChannel(read, retrier, resultRetryAlgorithm)
.setHasher(Hasher.defaultHasher())
.setHasher(Hasher.readHasher())
.setAutoGzipDecompression(autoGzipDecompression);
BufferHandle bufferHandle = getBufferHandle();
// because we're erasing the specific type of channel, we need to declare it here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,27 @@ final class DefaultInstanceHolder {
}
}

final class ReadInstanceHolder {
private static final Logger LOGGER = Logger.getLogger(Hasher.class.getName());
private static final String PROPERTY_NAME = "com.google.cloud.storage.Hasher.read";
private static final String PROPERTY_VALUE =
System.getProperty(PROPERTY_NAME, DefaultInstanceHolder.PROPERTY_VALUE);
static final Hasher READ_HASHER;

static {
LOGGER.fine(String.format(Locale.US, "-D%s=%s", PROPERTY_NAME, PROPERTY_VALUE));
if ("disabled".equalsIgnoreCase(PROPERTY_VALUE)) {
READ_HASHER = noop();
} else {
READ_HASHER = enabled();
}
}
}

static Hasher readHasher() {
return ReadInstanceHolder.READ_HASHER;
}

@Nullable
default Crc32cLengthKnown hash(Supplier<ByteBuffer> b) {
return hash(b.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static final class ReadableByteChannelSessionBuilder {

private ReadableByteChannelSessionBuilder(BlobReadChannelContext blobReadChannelContext) {
this.blobReadChannelContext = blobReadChannelContext;
this.hasher = Hasher.defaultHasher();
this.hasher = Hasher.readHasher();
this.autoGzipDecompression = false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public final class HttpStorageRpcHasherHelper {
private final Hasher hasher;

private HttpStorageRpcHasherHelper() {
hasher = Hasher.defaultHasher();
hasher = Hasher.readHasher();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
@BetaApi
@Immutable
public final class ReadAsChannel extends BaseConfig<ScatteringByteChannel, StreamingRead> {
static final ReadAsChannel INSTANCE = new ReadAsChannel(RangeSpec.all(), Hasher.enabled());
static final ReadAsChannel INSTANCE = new ReadAsChannel(RangeSpec.all(), Hasher.readHasher());

private final RangeSpec range;
private final Hasher hasher;
Expand Down Expand Up @@ -97,7 +97,7 @@ public ReadAsChannel withRangeSpec(RangeSpec range) {
*/
@BetaApi
boolean getCrc32cValidationEnabled() {
return Hasher.enabled().equals(hasher);
return !Hasher.noop().equals(hasher);
}

/**
Expand All @@ -111,7 +111,7 @@ boolean getCrc32cValidationEnabled() {
*/
@BetaApi
ReadAsChannel withCrc32cValidationEnabled(boolean enabled) {
if (enabled && Hasher.enabled().equals(hasher)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is did required?

if (enabled && !Hasher.noop().equals(hasher)) {
return this;
} else if (!enabled && Hasher.noop().equals(hasher)) {
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public final class ReadAsFutureByteString
extends BaseConfig<ApiFuture<DisposableByteString>, AccumulatingRead<DisposableByteString>> {

static final ReadAsFutureByteString INSTANCE =
new ReadAsFutureByteString(RangeSpec.all(), Hasher.enabled());
new ReadAsFutureByteString(RangeSpec.all(), Hasher.readHasher());

private final RangeSpec range;
private final Hasher hasher;
Expand Down Expand Up @@ -97,7 +97,7 @@ public ReadAsFutureByteString withRangeSpec(RangeSpec range) {
*/
@BetaApi
boolean getCrc32cValidationEnabled() {
return Hasher.enabled().equals(hasher);
return !Hasher.noop().equals(hasher);
}

/**
Expand All @@ -111,7 +111,7 @@ boolean getCrc32cValidationEnabled() {
*/
@BetaApi
ReadAsFutureByteString withCrc32cValidationEnabled(boolean enabled) {
if (enabled && Hasher.enabled().equals(hasher)) {
if (enabled && !Hasher.noop().equals(hasher)) {
return this;
} else if (!enabled && Hasher.noop().equals(hasher)) {
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public final class ReadAsFutureBytes
extends BaseConfig<ApiFuture<byte[]>, AccumulatingRead<byte[]>> {

static final ReadAsFutureBytes INSTANCE =
new ReadAsFutureBytes(RangeSpec.all(), Hasher.enabled());
new ReadAsFutureBytes(RangeSpec.all(), Hasher.readHasher());

private final RangeSpec range;
private final Hasher hasher;
Expand Down Expand Up @@ -92,7 +92,7 @@ public ReadAsFutureBytes withRangeSpec(RangeSpec range) {
*/
@BetaApi
boolean getCrc32cValidationEnabled() {
return Hasher.enabled().equals(hasher);
return !Hasher.noop().equals(hasher);
}

/**
Expand All @@ -106,7 +106,7 @@ boolean getCrc32cValidationEnabled() {
*/
@BetaApi
ReadAsFutureBytes withCrc32cValidationEnabled(boolean enabled) {
if (enabled && Hasher.enabled().equals(hasher)) {
if (enabled && !Hasher.noop().equals(hasher)) {
return this;
} else if (!enabled && Hasher.noop().equals(hasher)) {
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
public final class ReadAsSeekableChannel extends ReadProjectionConfig<SeekableByteChannel> {

static final ReadAsSeekableChannel INSTANCE =
new ReadAsSeekableChannel(Hasher.enabled(), LinearExponentialRangeSpecFunction.INSTANCE);
new ReadAsSeekableChannel(Hasher.readHasher(), LinearExponentialRangeSpecFunction.INSTANCE);

private final Hasher hasher;
private final RangeSpecFunction rangeSpecFunction;
Expand Down Expand Up @@ -94,7 +94,7 @@ public ReadAsSeekableChannel withRangeSpecFunction(RangeSpecFunction rangeSpecFu
*/
@BetaApi
boolean getCrc32cValidationEnabled() {
return Hasher.enabled().equals(hasher);
return !Hasher.noop().equals(hasher);
}

/**
Expand All @@ -108,7 +108,7 @@ boolean getCrc32cValidationEnabled() {
*/
@BetaApi
ReadAsSeekableChannel withCrc32cValidationEnabled(boolean enabled) {
if (enabled && Hasher.enabled().equals(hasher)) {
if (enabled && !Hasher.noop().equals(hasher)) {
return this;
} else if (!enabled && Hasher.noop().equals(hasher)) {
return this;
Expand Down
Loading