From 4b206a3540485a4a747df59ca127ef6d4305d4bd Mon Sep 17 00:00:00 2001
From: Mark DePristo <depristo@broadinstitute.org>
Date: Fri, 31 May 2013 13:54:33 -0400
Subject: [PATCH] Check that -compress arguments are within range 0-9

-- Although the original bug report was about SplitSamFile it actually was an engine wide error.  The two places in the that provide compression to the BAM write now check the validity of the compress argument via a static method in ReadUtils
-- delivers #49531009
---
 .../SAMFileWriterArgumentTypeDescriptor.java     |  7 ++++---
 .../sting/utils/sam/ReadUtils.java               |  8 ++++++++
 .../gatk/EngineFeaturesIntegrationTest.java      | 16 ++++++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/public/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java
index 458846db0..3b89787ad 100644
--- a/public/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java
+++ b/public/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java
@@ -30,6 +30,7 @@ import org.broadinstitute.sting.commandline.*;
 import org.broadinstitute.sting.gatk.GenomeAnalysisEngine;
 import org.broadinstitute.sting.gatk.io.StingSAMFileWriter;
 import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
+import org.broadinstitute.sting.utils.sam.ReadUtils;
 
 import java.io.OutputStream;
 import java.lang.annotation.Annotation;
@@ -132,9 +133,9 @@ public class SAMFileWriterArgumentTypeDescriptor extends ArgumentTypeDescriptor
         if (writerFileName != null &&  writerFileName.asFile() != null ) {
             stub = new SAMFileWriterStub(engine, writerFileName.asFile());
 
-            if ( compressionLevel != null )
-                stub.setCompressionLevel(compressionLevel);
-            if ( indexOnTheFly )
+            if ( compressionLevel != null ) {
+                stub.setCompressionLevel(ReadUtils.validateCompressionLevel(compressionLevel));
+            } if ( indexOnTheFly )
                 stub.setIndexOnTheFly(indexOnTheFly);
             if ( generateMD5 )
                 stub.setGenerateMD5(generateMD5);
diff --git a/public/java/src/org/broadinstitute/sting/utils/sam/ReadUtils.java b/public/java/src/org/broadinstitute/sting/utils/sam/ReadUtils.java
index 5b15fdd1b..cf1c9cb8e 100644
--- a/public/java/src/org/broadinstitute/sting/utils/sam/ReadUtils.java
+++ b/public/java/src/org/broadinstitute/sting/utils/sam/ReadUtils.java
@@ -36,6 +36,7 @@ import org.broadinstitute.sting.utils.MathUtils;
 import org.broadinstitute.sting.utils.NGSPlatform;
 import org.broadinstitute.sting.utils.collections.Pair;
 import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
+import org.broadinstitute.sting.utils.exceptions.UserException;
 
 import java.io.File;
 import java.util.*;
@@ -152,11 +153,18 @@ public class ReadUtils {
      * @return a SAMFileWriter with the compression level if it is a bam.
      */
     public static SAMFileWriter createSAMFileWriterWithCompression(SAMFileHeader header, boolean presorted, String file, int compression) {
+        validateCompressionLevel(compression);
         if (file.endsWith(".bam"))
             return new SAMFileWriterFactory().makeBAMWriter(header, presorted, new File(file), compression);
         return new SAMFileWriterFactory().makeSAMOrBAMWriter(header, presorted, new File(file));
     }
 
+    public static int validateCompressionLevel(final int requestedCompressionLevel) {
+        if ( requestedCompressionLevel < 0 || requestedCompressionLevel > 9 )
+            throw new UserException.BadArgumentValue("compress", "Compression level must be 0-9 but got " + requestedCompressionLevel);
+        return requestedCompressionLevel;
+    }
+
     /**
      * is this base inside the adaptor of the read?
      *
diff --git a/public/java/test/org/broadinstitute/sting/gatk/EngineFeaturesIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/EngineFeaturesIntegrationTest.java
index c60c6430c..6cfa90d90 100644
--- a/public/java/test/org/broadinstitute/sting/gatk/EngineFeaturesIntegrationTest.java
+++ b/public/java/test/org/broadinstitute/sting/gatk/EngineFeaturesIntegrationTest.java
@@ -174,4 +174,20 @@ public class EngineFeaturesIntegrationTest extends WalkerTest {
                 1, Arrays.asList("ecf27a776cdfc771defab1c5d19de9ab"));
         executeTest("testUserReadFilterAppliedBeforeWalker", spec);
     }
+
+    @Test
+    public void testNegativeCompress() {
+        testBadCompressArgument(-1);
+    }
+
+    @Test
+    public void testTooBigCompress() {
+        testBadCompressArgument(100);
+    }
+
+    private void testBadCompressArgument(final int compress) {
+        WalkerTestSpec spec = new WalkerTestSpec("-T PrintReads -R " + b37KGReference + " -I private/testdata/NA12878.1_10mb_2_10mb.bam -o %s -compress " + compress,
+                1, UserException.class);
+        executeTest("badCompress " + compress, spec);
+    }
 }
\ No newline at end of file