This is an automated email from the ASF dual-hosted git repository. brandonwilliams pushed a commit to branch cassandra-3.0 in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-3.0 by this push: new 4886968 Fix chunk index overflow due to large sstable with small chunk length 4886968 is described below commit 4886968c4e973488df4f2da480785beff09b562b Author: Zhao Yang <zhaoyangsingap...@gmail.com> AuthorDate: Mon Apr 27 01:09:27 2020 +0800 Fix chunk index overflow due to large sstable with small chunk length patch by ZhaoYang; reviewed by brandonwilliams for CASSANDRA-15595 --- CHANGES.txt | 1 + .../cassandra/io/compress/CompressionMetadata.java | 6 +- .../compress/CompressedRandomAccessReaderTest.java | 100 +++++++++++++++------ 3 files changed, 80 insertions(+), 27 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index efe35a7..b906f15 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,6 @@ 3.0.21 * Fix infinite loop on index query paging in tables with clustering (CASSANDRA-14242) + * Fix chunk index overflow due to large sstable with small chunk length (CASSANDRA-15595) * 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) * Memtable memory allocations may deadlock (CASSANDRA-15367) diff --git a/src/java/org/apache/cassandra/io/compress/CompressionMetadata.java b/src/java/org/apache/cassandra/io/compress/CompressionMetadata.java index 101f722..10d1ae9 100644 --- a/src/java/org/apache/cassandra/io/compress/CompressionMetadata.java +++ b/src/java/org/apache/cassandra/io/compress/CompressionMetadata.java @@ -227,11 +227,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 0c96327..34ff94f 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; @@ -43,6 +44,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 { @@ -75,11 +77,11 @@ public class CompressedRandomAccessReaderTest { File f = File.createTempFile("compressed6791_", "3"); String filename = f.getAbsolutePath(); - try(ChannelProxy channel = new ChannelProxy(f)) + try (ChannelProxy channel = new ChannelProxy(f)) { MetadataCollector sstableMetadataCollector = new MetadataCollector(new ClusteringComparator(BytesType.instance)); - try(CompressedSequentialWriter writer = new CompressedSequentialWriter(f, filename + ".metadata", CompressionParams.snappy(32), sstableMetadataCollector)) + try (CompressedSequentialWriter writer = new CompressedSequentialWriter(f, filename + ".metadata", CompressionParams.snappy(32), sstableMetadataCollector)) { for (int i = 0; i < 20; i++) @@ -97,9 +99,9 @@ public class CompressedRandomAccessReaderTest writer.finish(); } - try(RandomAccessReader reader = new CompressedRandomAccessReader.Builder(channel, - new CompressionMetadata(filename + ".metadata", f.length(), ChecksumType.CRC32)) - .build()) + try (RandomAccessReader reader = new CompressedRandomAccessReader.Builder(channel, + new CompressionMetadata(filename + ".metadata", f.length(), ChecksumType.CRC32)) + .build()) { String res = reader.readLine(); assertEquals(res, "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); @@ -110,37 +112,58 @@ public class CompressedRandomAccessReaderTest { if (f.exists()) assertTrue(f.delete()); - File metadata = new File(filename+ ".metadata"); + 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 + /** + * JIRA: CASSANDRA-15595 verify large position with small chunk length won't overflow chunk index + */ + @Test + public void testChunkIndexOverflow() throws IOException { - final String filename = f.getAbsolutePath(); - try(ChannelProxy channel = new ChannelProxy(f)) + File file = File.createTempFile("chunk_idx_overflow", "1"); + String filename = file.getAbsolutePath(); + int chunkLength = 4096; // 4k + + try { - MetadataCollector sstableMetadataCollector = new MetadataCollector(new ClusteringComparator(BytesType.instance)); - try(SequentialWriter writer = compressed - ? new CompressedSequentialWriter(f, filename + ".metadata", CompressionParams.snappy(), sstableMetadataCollector) - : SequentialWriter.open(f)) - { - writer.write("The quick ".getBytes()); - DataPosition mark = writer.mark(); - writer.write("blue fox jumps over the lazy dog".getBytes()); + writeSSTable(file, CompressionParams.snappy(chunkLength), 10); + CompressionMetadata metadata = new CompressionMetadata(filename + ".metadata", file.length(), ChecksumType.CRC32); - // write enough to be sure to change chunk - for (int i = 0; i < junkSize; ++i) - { - writer.write((byte) 1); - } + long chunks = 2761628520L; + long midPosition = (chunks / 2L) * chunkLength; + int idx = 8 * (int) (midPosition / chunkLength); // before patch + assertTrue("Expect integer overflow", idx < 0); - writer.resetAndTruncate(mark); - writer.write("brown fox jumps over the lazy dog".getBytes()); - writer.finish(); + try + { + metadata.chunkFor(midPosition); + fail("Expected to throw EOF exception with chunk idx larger than total number of chunks in the sstable"); } - assert f.exists(); + 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(); + try(ChannelProxy channel = new ChannelProxy(f)) + { + writeSSTable(f, compressed ? CompressionParams.snappy() : null, junkSize); CompressionMetadata compressionMetadata = compressed ? new CompressionMetadata(filename + ".metadata", f.length(), ChecksumType.CRC32) : null; RandomAccessReader.Builder builder = compressed @@ -177,6 +200,31 @@ 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", params, sstableMetadataCollector) + : SequentialWriter.open(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 */ --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org