Disallow tabix indices for non-BGZIP inputs

src/main/java/htsjdk/tribble/AbstractFeatureReader.java

@@ -18,6 +18,9 @@
 
 package htsjdk.tribble;
 
+import htsjdk.samtools.seekablestream.SeekableStream;
+import htsjdk.samtools.seekablestream.SeekableStreamFactory;
+import htsjdk.samtools.util.BlockCompressedInputStream;
 import htsjdk.tribble.index.Index;
 import htsjdk.tribble.util.ParsingUtils;
 import htsjdk.tribble.util.TabixUtils;
@@ -26,11 +29,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.nio.channels.SeekableByteChannel;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Set;
+import java.util.*;
 import java.util.function.Function;
 
 /**
@@ -99,8 +98,8 @@ public static <FEATURE extends Feature, SOURCE> AbstractFeatureReader<FEATURE, S
      */
     public static <FEATURE extends Feature, SOURCE> AbstractFeatureReader<FEATURE, SOURCE> getFeatureReader(final String featureResource, String indexResource, final FeatureCodec<FEATURE, SOURCE> codec, final boolean requireIndex, Function<SeekableByteChannel, SeekableByteChannel> wrapper, Function<SeekableByteChannel, SeekableByteChannel> indexWrapper) throws TribbleException {
         try {
-            // Test for tabix index
-            if (methods.isTabix(featureResource, indexResource)) {
+            // Test for BGZF formatted file
+            if (methods.isTabix(featureResource, SeekableStreamFactory.getInstance().getBufferedStream(SeekableStreamFactory.getInstance().getStreamFor(featureResource, wrapper)), indexResource)) {
                 if ( ! (codec instanceof AsciiFeatureCodec) )
                     throw new TribbleException("Tabix indexed files only work with ASCII codecs, but received non-Ascii codec " + codec.getClass().getSimpleName());
                 return new TabixFeatureReader<>(featureResource, indexResource, (AsciiFeatureCodec) codec, wrapper, indexWrapper);
@@ -237,16 +236,42 @@ static class EmptyIterator<T extends Feature> implements CloseableTribbleIterato
     }
 
     public static boolean isTabix(String resourcePath, String indexPath) throws IOException {
-        if(indexPath == null){
+        try (SeekableStream inputStream = SeekableStreamFactory.getInstance().getStreamFor(resourcePath)) {
+            return isTabix(resourcePath, inputStream,indexPath);
+        } catch (IOException e) {
+            return false;
+        }
+    }
+
+    public static boolean isTabix(String resourcePath, SeekableStream inputStream, String indexPath) throws IOException {
+        if (indexPath == null) {
             indexPath = ParsingUtils.appendToPath(resourcePath, TabixUtils.STANDARD_INDEX_EXTENSION);
         }
-        return hasBlockCompressedExtension(resourcePath) && ParsingUtils.resourceExists(indexPath);
+        boolean isBGZF = isBGZFFile(inputStream);
+        boolean hasIndex = ParsingUtils.resourceExists(indexPath);
+        if (hasIndex && !isBGZF && indexPath.endsWith(TabixUtils.STANDARD_INDEX_EXTENSION)) {
+            throw new TribbleException(String.format("Detected Tabix index for file %s, but the file does not appear to be compressed in BGZF format", resourcePath));
+        }
+
+        return hasBlockCompressedExtension(resourcePath) && hasIndex;
+    }
+
+    public static boolean isBGZFFile(SeekableStream inputStream) {
+        try {
+            return BlockCompressedInputStream.isValidFile(inputStream);
+        } catch (IOException e) {
+            return false;
+        }
     }
 
     public static class ComponentMethods{
 
         public boolean isTabix(String resourcePath, String indexPath) throws IOException{
             return AbstractFeatureReader.isTabix(resourcePath, indexPath);
         }
+
+        public boolean isTabix(String resourcePath, SeekableStream inputStream, String indexPath) throws IOException{
+            return AbstractFeatureReader.isTabix(resourcePath, inputStream, indexPath);
+        }
     }
 }

src/main/java/htsjdk/tribble/TribbleIndexedFeatureReader.java

@@ -25,6 +25,7 @@
 
 import htsjdk.samtools.seekablestream.SeekableStream;
 import htsjdk.samtools.seekablestream.SeekableStreamFactory;
+import htsjdk.samtools.util.BlockCompressedInputStream;
 import htsjdk.samtools.util.RuntimeIOException;
 import htsjdk.tribble.index.Block;
 import htsjdk.tribble.index.Index;
@@ -321,13 +322,15 @@ class WFIterator implements CloseableTribbleIterator<T> {
          * @throws IOException
          */
         public WFIterator() throws IOException {
-            final InputStream inputStream = ParsingUtils.openInputStream(path, wrapper);
+            final InputStream inputStream = new BufferedInputStream(ParsingUtils.openInputStream(path, wrapper), 512000);
 
             final PositionalBufferedStream pbs;
             if (hasBlockCompressedExtension(path)) {
                 // Gzipped -- we need to buffer the GZIPInputStream methods as this class makes read() calls,
                 // and seekableStream does not support single byte reads
-                final InputStream is = new GZIPInputStream(new BufferedInputStream(inputStream, 512000));
+                final InputStream is = BlockCompressedInputStream.isValidFile(inputStream) ?
+                        new BlockCompressedInputStream(inputStream) :
+                        new GZIPInputStream(inputStream);
                 pbs = new PositionalBufferedStream(is, 1000);  // Small buffer as this is buffered already.
             } else {
                 pbs = new PositionalBufferedStream(inputStream, 512000);

src/test/java/htsjdk/tribble/AbstractFeatureReaderTest.java

@@ -42,7 +42,9 @@ public class AbstractFeatureReaderTest extends HtsjdkTest {
     private static final String VCF = TEST_PATH + "baseVariants.vcf";
     private static final String VCF_INDEX = TEST_PATH + "baseVariants.vcf.idx";
     private static final String VCF_TABIX_BLOCK_GZIPPED = TEST_PATH + "baseVariants.vcf.gz";
+    private static final String VCF_TABIX_BLOCK_GZIPPED_NOINDEX = TEST_PATH + "baseVariants.noIndex.vcf.gz";
     private static final String VCF_TABIX_INDEX = TEST_PATH + "baseVariants.vcf.gz.tbi";
+    private static final String VCF_TABIX_NONBLOCK_GZIPPED = TEST_PATH + "baseVariants.nonBlockCompressed.vcf.gz";
     private static final String MANGLED_VCF_TABIX_BLOCK_GZIPPED = TEST_PATH + "baseVariants.mangled.vcf.gz";
     private static final String MANGLED_VCF_TABIX_INDEX = TEST_PATH + "baseVariants.mangled.vcf.gz.tbi";
     private static final String CORRUPTED_VCF_INDEX = TEST_PATH + "corruptedBaseVariants.vcf.idx";
@@ -141,30 +143,36 @@ public void testBlockCompressionExtensionStringVersion(final String testURIStrin
     @DataProvider(name = "vcfFileAndWrapperCombinations")
     private static Object[][] vcfFileAndWrapperCombinations(){
         return new Object[][] {
-                {VCF, VCF_INDEX, null, null},
-                {MANGLED_VCF, MANGLED_VCF_INDEX, WRAPPER, WRAPPER},
-                {VCF, MANGLED_VCF_INDEX, null, WRAPPER},
-                {MANGLED_VCF, VCF_INDEX, WRAPPER, null},
-                {MANGLED_VCF_TABIX_BLOCK_GZIPPED, MANGLED_VCF_TABIX_INDEX, WRAPPER, WRAPPER},
-                {VCF_TABIX_BLOCK_GZIPPED, MANGLED_VCF_TABIX_INDEX, null, WRAPPER},
-                {MANGLED_VCF_TABIX_BLOCK_GZIPPED, VCF_TABIX_INDEX, WRAPPER, null},
-                {VCF_TABIX_BLOCK_GZIPPED, VCF_TABIX_INDEX, null, null},
+                {VCF, VCF_INDEX, null, null, false},
+                {MANGLED_VCF, MANGLED_VCF_INDEX, WRAPPER, WRAPPER, false},
+                {VCF, MANGLED_VCF_INDEX, null, WRAPPER, false},
+                {MANGLED_VCF, VCF_INDEX, WRAPPER, null, false},
+                {MANGLED_VCF_TABIX_BLOCK_GZIPPED, MANGLED_VCF_TABIX_INDEX, WRAPPER, WRAPPER, true},
+                {VCF_TABIX_BLOCK_GZIPPED, MANGLED_VCF_TABIX_INDEX, null, WRAPPER, true},
+                {MANGLED_VCF_TABIX_BLOCK_GZIPPED, VCF_TABIX_INDEX, WRAPPER, null, true},
+                {VCF_TABIX_BLOCK_GZIPPED, VCF_TABIX_INDEX, null, null, true},
+                {VCF_TABIX_BLOCK_GZIPPED_NOINDEX, null, null, null, false},
+                {VCF_TABIX_NONBLOCK_GZIPPED, null, null, null, false},
         };
     }
 
     @Test(dataProvider = "vcfFileAndWrapperCombinations")
     public void testGetFeatureReaderWithPathAndWrappers(String file, String index,
                                                         Function<SeekableByteChannel, SeekableByteChannel> wrapper,
-                                                        Function<SeekableByteChannel, SeekableByteChannel> indexWrapper) throws IOException, URISyntaxException {
-        try(FileSystem fs = Jimfs.newFileSystem("test", Configuration.unix());
-            final AbstractFeatureReader<VariantContext, ?> featureReader = getFeatureReader(file, index, wrapper,
-                                                                                            indexWrapper,
-                                                                                            new VCFCodec(),
-                                                                                            fs)){
-            Assert.assertTrue(featureReader.hasIndex());
-            Assert.assertEquals(featureReader.iterator().toList().size(), 26);
-            Assert.assertEquals(featureReader.query("1", 190, 210).toList().size(), 3);
-            Assert.assertEquals(featureReader.query("2", 190, 210).toList().size(), 1);
+                                                        Function<SeekableByteChannel, SeekableByteChannel> indexWrapper,
+                                                        boolean useTribbleReader) throws IOException, URISyntaxException {
+        if (index!=null) {
+            try (FileSystem fs = Jimfs.newFileSystem("test", Configuration.unix());
+                 final AbstractFeatureReader<VariantContext, ?> featureReader = getFeatureReader(file, index, wrapper,
+                         indexWrapper,
+                         new VCFCodec(),
+                         fs,
+                         false)) {
+                Assert.assertTrue(featureReader.hasIndex());
+                Assert.assertEquals(featureReader.iterator().toList().size(), 26);
+                Assert.assertEquals(featureReader.query("1", 190, 210).toList().size(), 3);
+                Assert.assertEquals(featureReader.query("2", 190, 210).toList().size(), 1);
+            }
         }
     }
 
@@ -178,13 +186,14 @@ public static Object[][] failsWithoutWrappers(){
                 {MANGLED_VCF_TABIX_BLOCK_GZIPPED, MANGLED_VCF_TABIX_INDEX},
                 {VCF_TABIX_BLOCK_GZIPPED, MANGLED_VCF_TABIX_INDEX},
                 {MANGLED_VCF_TABIX_BLOCK_GZIPPED, VCF_TABIX_INDEX},
+                {VCF_TABIX_NONBLOCK_GZIPPED, VCF_TABIX_BLOCK_GZIPPED} // Asserting that there is an error when a tbi is present for a non-BGZF file
         };
     }
 
     @Test(dataProvider = "failsWithoutWrappers", expectedExceptions = {TribbleException.class, FileTruncatedException.class})
     public void testFailureIfNoWrapper(String file, String index) throws IOException, URISyntaxException {
         try(final FileSystem fs = Jimfs.newFileSystem("test", Configuration.unix());
-            final FeatureReader<?> reader = getFeatureReader(file, index, null, null, new VCFCodec(), fs)){
+            final FeatureReader<?> reader = getFeatureReader(file, index, null, null, new VCFCodec(), fs, true)){
             // should have exploded by now
         }
     }
@@ -193,17 +202,43 @@ public void testFailureIfNoWrapper(String file, String index) throws IOException
                                                                                     Function<SeekableByteChannel, SeekableByteChannel> wrapper,
                                                                                     Function<SeekableByteChannel, SeekableByteChannel> indexWrapper,
                                                                                     FeatureCodec<T, ?> codec,
-                                                                                    FileSystem fileSystem) throws IOException, URISyntaxException {
-        final Path vcfInJimfs = TestUtils.getTribbleFileInJimfs(vcf, index, fileSystem);
+                                                                                    FileSystem fileSystem,
+                                                                                    boolean requireIndex) throws IOException, URISyntaxException {
+        final Path vcfInJimfs = TestUtils.getTribbleFileInJimfs(vcf, wrapper, index, fileSystem);
         return AbstractFeatureReader.getFeatureReader(
                 vcfInJimfs.toUri().toString(),
                 null,
                 codec,
-                true,
+                requireIndex,
                 wrapper,
                 indexWrapper);
     }
 
+    @Test
+    public void testIsTabix() throws IOException {
+        Assert.assertFalse(AbstractFeatureReader.isTabix(MANGLED_VCF, null));
+        Assert.assertFalse(AbstractFeatureReader.isTabix(VCF, null));
+        Assert.assertFalse(AbstractFeatureReader.isTabix(VCF, VCF_INDEX));
+
+        Assert.assertTrue(AbstractFeatureReader.isTabix(VCF_TABIX_BLOCK_GZIPPED, null));
+        Assert.assertTrue(AbstractFeatureReader.isTabix(VCF_TABIX_BLOCK_GZIPPED, VCF_TABIX_INDEX));
+        Assert.assertFalse(AbstractFeatureReader.isTabix(VCF_TABIX_BLOCK_GZIPPED_NOINDEX, null));
+    }
+
+    @Test(dataProvider = "vcfFileAndWrapperCombinations")
+    public void testUsingTabixInputStream(String file, String index,
+                                                        Function<SeekableByteChannel, SeekableByteChannel> wrapper,
+                                                        Function<SeekableByteChannel, SeekableByteChannel> indexWrapper,
+                                                        boolean useTabixReader) throws IOException, URISyntaxException {
+        try(FileSystem fs = Jimfs.newFileSystem("test", Configuration.unix());
+            final AbstractFeatureReader<VariantContext, ?> featureReader = getFeatureReader(file, index, wrapper,
+                    indexWrapper,
+                    new VCFCodec(),
+                    fs,
+                    false)) {
+            Assert.assertEquals(featureReader instanceof TabixFeatureReader, useTabixReader);
+        }
+    }
     /**
      * skip the first byte of a SeekableByteChannel
      */

src/test/java/htsjdk/tribble/TestUtils.java

@@ -18,12 +18,18 @@
 
 package htsjdk.tribble;
 
+import htsjdk.samtools.seekablestream.SeekableStreamFactory;
+import htsjdk.tribble.util.ParsingUtils;
+import htsjdk.tribble.util.TabixUtils;
+
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.nio.channels.SeekableByteChannel;
 import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.function.Function;
 
 /**
  * User: jacob
@@ -44,17 +50,24 @@ public class TestUtils {
      * @throws URISyntaxException if the provided strings cannot be understoos as Uris.
      */
 
-    public static Path getTribbleFileInJimfs(String vcf, String index, FileSystem fileSystem) throws IOException, URISyntaxException {
+    public static Path getTribbleFileInJimfs(String vcf, Function<SeekableByteChannel, SeekableByteChannel> wrapper, String index, FileSystem fileSystem) throws IOException, URISyntaxException {
         final FileSystem fs = fileSystem;
         final Path root = fs.getPath("/");
         final Path vcfPath = Paths.get(vcf);
 
         final Path vcfDestination = root.resolve(vcfPath.getFileName().toString());
         if (index != null) {
             final Path idxPath = Paths.get(index);
-            final Path idxDestination = AbstractFeatureReader.isTabix(vcf, index) ? Tribble.tabixIndexPath(vcfDestination) : Tribble.indexPath(vcfDestination);
+            final Path idxDestination = isTabix(vcf, index) ? Tribble.tabixIndexPath(vcfDestination) : Tribble.indexPath(vcfDestination);
             Files.copy(idxPath, idxDestination);
         }
         return Files.copy(vcfPath, vcfDestination);
     }
+
+    private static boolean isTabix(String resourcePath, String indexPath) throws IOException {
+        if (indexPath == null) {
+            indexPath = ParsingUtils.appendToPath(resourcePath, TabixUtils.STANDARD_INDEX_EXTENSION);
+        }
+        return AbstractFeatureReader.hasBlockCompressedExtension(resourcePath) && ParsingUtils.resourceExists(indexPath);
+    }
 }

src/test/java/htsjdk/tribble/TribbleIndexFeatureReaderTest.java

@@ -38,4 +38,20 @@ public void testIndexedGZIPVCF(final String testPath, final int expectedCount) t
         }
     }
 
+    @Test
+    // This tests a large Unblocked GZipped vcf file which should fail to parse for a large input if it tries to open a BlockCompressedInputStream over the file
+    public void testGZIPVCFNotTabix() throws IOException {
+        final VCFCodec codec = new VCFCodec();
+        try (final TribbleIndexedFeatureReader<VariantContext, LineIterator> featureReader =
+                     new TribbleIndexedFeatureReader<>(TestUtils.DATA_DIR + "tabix/YRI.trio.2010_07.indel.sites.unBlocked.vcf.gz", codec, false)) {
+            final CloseableTribbleIterator<VariantContext> localIterator = featureReader.iterator();
+            int count = 0;
+            for (final Feature feat : featureReader.iterator()) {
+                localIterator.next();
+                count++;
+            }
+            Assert.assertEquals(count, 12218);
+        }
+    }
+
 }

src/test/java/htsjdk/variant/vcf/VCFFileReaderTest.java

@@ -75,7 +75,7 @@ Object[][] pathsData() {
     @Test(dataProvider = "pathsData", timeOut = 60_000)
     public void testCanOpenVCFPathReader(final String file, final String index, final boolean requiresIndex, final boolean shouldSucceed) throws Exception {
         try (FileSystem fs = Jimfs.newFileSystem("test", Configuration.unix())) {
-            final Path tribbleFileInJimfs = TestUtils.getTribbleFileInJimfs(file, index, fs);
+            final Path tribbleFileInJimfs = TestUtils.getTribbleFileInJimfs(file, null, index, fs);
             try (final VCFFileReader reader = new VCFFileReader(tribbleFileInJimfs, requiresIndex)) {
                 final VCFHeader header = reader.getFileHeader();
             } catch (Exception e) {