Skip to content

Commit b4ff4d0

Browse files
committed
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 00e8789 commit b4ff4d0

File tree

1 file changed

+16
-17
lines changed

1 file changed

+16
-17
lines changed

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

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,19 @@
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;
26-
import org.elasticsearch.index.translog.BufferedChecksumStreamOutput;
2725
import org.elasticsearch.repositories.blobstore.ChecksumBlobStoreFormat;
2826
import org.elasticsearch.test.ESTestCase;
2927

3028
import java.io.EOFException;
3129
import java.io.IOException;
3230
import java.io.InputStream;
3331
import java.util.Map;
32+
3433
import static org.hamcrest.Matchers.containsString;
3534
import static org.hamcrest.Matchers.greaterThan;
3635

@@ -145,24 +144,24 @@ protected BlobStore createTestBlobStore() throws IOException {
145144
}
146145

147146
protected void randomCorruption(BlobContainer blobContainer, String blobName) throws IOException {
148-
byte[] buffer = new byte[(int) blobContainer.listBlobsByPrefix(blobName).get(blobName).length()];
149-
long originalChecksum = checksum(buffer);
147+
final byte[] buffer = new byte[(int) blobContainer.listBlobsByPrefix(blobName).get(blobName).length()];
150148
try (InputStream inputStream = blobContainer.readBlob(blobName)) {
151149
Streams.readFully(inputStream, buffer);
152150
}
153-
do {
154-
int location = randomIntBetween(0, buffer.length - 1);
155-
buffer[location] = (byte) (buffer[location] ^ 42);
156-
} while (originalChecksum == checksum(buffer));
157-
blobContainer.writeBlob(blobName, new BytesArray(buffer), false);
158-
}
159-
160-
private long checksum(byte[] buffer) throws IOException {
161-
try (BytesStreamOutput streamOutput = new BytesStreamOutput()) {
162-
try (BufferedChecksumStreamOutput checksumOutput = new BufferedChecksumStreamOutput(streamOutput)) {
163-
checksumOutput.write(buffer);
164-
return checksumOutput.getChecksum();
165-
}
151+
final BytesArray corruptedBytes;
152+
final int location = randomIntBetween(0, buffer.length - 1);
153+
if (randomBoolean()) {
154+
// flipping bits in a single byte will always invalidate the file: CRC-32 certainly detects all eight-bit-burst errors; we don't
155+
// 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
156+
// be a detectable error:
157+
buffer[location] = (byte) (buffer[location] ^ between(1, 255));
158+
corruptedBytes = new BytesArray(buffer);
159+
} else {
160+
// truncation will invalidate the file: the last 12 bytes should start with 8 zero bytes but by construction we won't have
161+
// another sequence of 8 zero bytes anywhere in the file, let alone such a sequence followed by a correct checksum.
162+
corruptedBytes = new BytesArray(buffer, 0, location);
166163
}
164+
blobContainer.writeBlob(blobName, corruptedBytes, false);
167165
}
166+
168167
}

0 commit comments

Comments
 (0)