diff --git a/CHANGES.txt b/CHANGES.txt index fd7ad6a7a9..b3c593b830 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,7 @@ 3.11.7 * Allow sstableloader to use SSL on the native port (CASSANDRA-14904) Merged from 3.0: + * Fix chunk index overflow due to large sstable with small chunk length (CASSANDRA-15595) * Allow selecting static column only when querying static index (CASSANDRA-14242) * cqlsh return non-zero status when STDIN CQL fails (CASSANDRA-15623) * Don't skip sstables in slice queries based only on local min/max/deletion timestamp (CASSANDRA-15690) diff --git a/src/java/org/apache/cassandra/io/compress/CompressionMetadata.java b/src/java/org/apache/cassandra/io/compress/CompressionMetadata.java index 9a4d9199b2..90ba749a20 100644 --- a/src/java/org/apache/cassandra/io/compress/CompressionMetadata.java +++ b/src/java/org/apache/cassandra/io/compress/CompressionMetadata.java @@ -226,11 +226,15 @@ public class CompressionMetadata public Chunk chunkFor(long position) { // position of the chunk - int idx = 8 * (int) (position / parameters.chunkLength()); + long idx = 8 * (position / parameters.chunkLength()); if (idx >= chunkOffsetsSize) throw new CorruptSSTableException(new EOFException(), indexFilePath); + if (idx < 0) + throw new CorruptSSTableException(new IllegalArgumentException(String.format("Invalid negative chunk index %d with position %d", idx, position)), + indexFilePath); + long chunkOffset = chunkOffsets.getLong(idx); long nextChunkOffset = (idx + 8 == chunkOffsetsSize) ? compressedFileLength diff --git a/test/unit/org/apache/cassandra/io/compress/CompressedRandomAccessReaderTest.java b/test/unit/org/apache/cassandra/io/compress/CompressedRandomAccessReaderTest.java index 0d2a9fbcd4..c71814740d 100644 --- a/test/unit/org/apache/cassandra/io/compress/CompressedRandomAccessReaderTest.java +++ b/test/unit/org/apache/cassandra/io/compress/CompressedRandomAccessReaderTest.java @@ -18,6 +18,7 @@ */ package org.apache.cassandra.io.compress; +import java.io.EOFException; import java.io.File; import java.io.IOException; import java.io.RandomAccessFile; @@ -42,6 +43,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class CompressedRandomAccessReaderTest { @@ -123,31 +125,50 @@ public class CompressedRandomAccessReaderTest } } + /** + * JIRA: CASSANDRA-15595 verify large position with small chunk length won't overflow chunk index + */ + @Test + public void testChunkIndexOverflow() throws IOException + { + File file = File.createTempFile("chunk_idx_overflow", "1"); + String filename = file.getAbsolutePath(); + int chunkLength = 4096; // 4k + + try + { + writeSSTable(file, CompressionParams.snappy(chunkLength), 10); + CompressionMetadata metadata = new CompressionMetadata(filename + ".metadata", file.length(), ChecksumType.CRC32); + + long chunks = 2761628520L; + long midPosition = (chunks / 2L) * chunkLength; + int idx = 8 * (int) (midPosition / chunkLength); // before patch + assertTrue("Expect integer overflow", idx < 0); + + try + { + metadata.chunkFor(midPosition); + fail("Expected to throw EOF exception with chunk idx larger than total number of chunks in the sstable"); + } + catch (CorruptSSTableException e) + { + assertTrue("Expect EOF, but got " + e.getCause(), e.getCause() instanceof EOFException); + } + } + finally + { + if (file.exists()) + assertTrue(file.delete()); + File metadata = new File(filename + ".metadata"); + if (metadata.exists()) + metadata.delete(); + } + } + private static void testResetAndTruncate(File f, boolean compressed, boolean usemmap, int junkSize) throws IOException { final String filename = f.getAbsolutePath(); - MetadataCollector sstableMetadataCollector = new MetadataCollector(new ClusteringComparator(BytesType.instance)); - try(SequentialWriter writer = compressed - ? new CompressedSequentialWriter(f, filename + ".metadata", - null, SequentialWriterOption.DEFAULT, - CompressionParams.snappy(), sstableMetadataCollector) - : new SequentialWriter(f)) - { - writer.write("The quick ".getBytes()); - DataPosition mark = writer.mark(); - writer.write("blue fox jumps over the lazy dog".getBytes()); - - // write enough to be sure to change chunk - for (int i = 0; i < junkSize; ++i) - { - writer.write((byte) 1); - } - - writer.resetAndTruncate(mark); - writer.write("brown fox jumps over the lazy dog".getBytes()); - writer.finish(); - } - assert f.exists(); + writeSSTable(f, compressed ? CompressionParams.snappy() : null, junkSize); CompressionMetadata compressionMetadata = compressed ? new CompressionMetadata(filename + ".metadata", f.length(), ChecksumType.CRC32) : null; try (FileHandle.Builder builder = new FileHandle.Builder(filename).mmapped(usemmap).withCompressionMetadata(compressionMetadata); @@ -170,6 +191,33 @@ public class CompressedRandomAccessReaderTest } } + private static void writeSSTable(File f, CompressionParams params, int junkSize) throws IOException + { + final String filename = f.getAbsolutePath(); + MetadataCollector sstableMetadataCollector = new MetadataCollector(new ClusteringComparator(BytesType.instance)); + try(SequentialWriter writer = params != null + ? new CompressedSequentialWriter(f, filename + ".metadata", + null, SequentialWriterOption.DEFAULT, + params, sstableMetadataCollector) + : new SequentialWriter(f)) + { + writer.write("The quick ".getBytes()); + DataPosition mark = writer.mark(); + writer.write("blue fox jumps over the lazy dog".getBytes()); + + // write enough to be sure to change chunk + for (int i = 0; i < junkSize; ++i) + { + writer.write((byte) 1); + } + + writer.resetAndTruncate(mark); + writer.write("brown fox jumps over the lazy dog".getBytes()); + writer.finish(); + } + assert f.exists(); + } + /** * If the data read out doesn't match the checksum, an exception should be thrown */