Skip to content

Commit 7a0eaab

Browse files
authored
Improve BlobStoreFormatTests#randomCorruption (#73201)
This method today corrupts bytes until the checksum changes, but (a) it's comparing the checksum vs one computed before even reading the file, and (b) changing a single byte will always invalidate a CRC-32 checksum so the loop is unnecessary as is the checksum calculation. It also doesn't ever try truncating the file which is a realistic kind of corruption that we must be able to detect. This commit addresses all that.
1 parent 8e46acf commit 7a0eaab

File tree

1 file changed

+15
-17
lines changed

1 file changed

+15
-17
lines changed

server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatTests.java

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,12 @@
1717
import org.elasticsearch.common.blobstore.fs.FsBlobStore;
1818
import org.elasticsearch.common.bytes.BytesArray;
1919
import org.elasticsearch.common.io.Streams;
20-
import org.elasticsearch.common.io.stream.BytesStreamOutput;
2120
import org.elasticsearch.common.util.MockBigArrays;
2221
import org.elasticsearch.common.xcontent.ToXContent;
2322
import org.elasticsearch.common.xcontent.ToXContentFragment;
2423
import org.elasticsearch.common.xcontent.XContentBuilder;
2524
import org.elasticsearch.common.xcontent.XContentParser;
2625
import org.elasticsearch.common.xcontent.XContentParserUtils;
27-
import org.elasticsearch.index.translog.BufferedChecksumStreamOutput;
2826
import org.elasticsearch.repositories.blobstore.ChecksumBlobStoreFormat;
2927
import org.elasticsearch.test.ESTestCase;
3028

@@ -134,24 +132,24 @@ protected BlobStore createTestBlobStore() throws IOException {
134132
}
135133

136134
protected void randomCorruption(BlobContainer blobContainer, String blobName) throws IOException {
137-
byte[] buffer = new byte[(int) blobContainer.listBlobsByPrefix(blobName).get(blobName).length()];
138-
long originalChecksum = checksum(buffer);
135+
final byte[] buffer = new byte[(int) blobContainer.listBlobsByPrefix(blobName).get(blobName).length()];
139136
try (InputStream inputStream = blobContainer.readBlob(blobName)) {
140137
Streams.readFully(inputStream, buffer);
141138
}
142-
do {
143-
int location = randomIntBetween(0, buffer.length - 1);
144-
buffer[location] = (byte) (buffer[location] ^ 42);
145-
} while (originalChecksum == checksum(buffer));
146-
blobContainer.writeBlob(blobName, new BytesArray(buffer), false);
147-
}
148-
149-
private long checksum(byte[] buffer) throws IOException {
150-
try (BytesStreamOutput streamOutput = new BytesStreamOutput()) {
151-
try (BufferedChecksumStreamOutput checksumOutput = new BufferedChecksumStreamOutput(streamOutput)) {
152-
checksumOutput.write(buffer);
153-
return checksumOutput.getChecksum();
154-
}
139+
final BytesArray corruptedBytes;
140+
final int location = randomIntBetween(0, buffer.length - 1);
141+
if (randomBoolean()) {
142+
// flipping bits in a single byte will always invalidate the file: CRC-32 certainly detects all eight-bit-burst errors; we don't
143+
// checksum the last 8 bytes but we do verify that they contain the checksum preceded by 4 zero bytes so in any case this will
144+
// be a detectable error:
145+
buffer[location] = (byte) (buffer[location] ^ between(1, 255));
146+
corruptedBytes = new BytesArray(buffer);
147+
} else {
148+
// truncation will invalidate the file: the last 12 bytes should start with 8 zero bytes but by construction we won't have
149+
// another sequence of 8 zero bytes anywhere in the file, let alone such a sequence followed by a correct checksum.
150+
corruptedBytes = new BytesArray(buffer, 0, location);
155151
}
152+
blobContainer.writeBlob(blobName, corruptedBytes, false);
156153
}
154+
157155
}

0 commit comments

Comments
 (0)