Browse Source

HADOOP-17877. BuiltInGzipCompressor header and trailer should not be static variables (#3350)

Liang-Chi Hsieh 3 years ago
parent
commit
73a0c31370

+ 18 - 18
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/zlib/BuiltInGzipCompressor.java

@@ -39,15 +39,15 @@ public class BuiltInGzipCompressor implements Compressor {
    * Fixed ten-byte gzip header. See {@link GZIPOutputStream}'s source for
    * Fixed ten-byte gzip header. See {@link GZIPOutputStream}'s source for
    * details.
    * details.
    */
    */
-  private static final byte[] GZIP_HEADER = new byte[]{
+  private final byte[] gzipHeader = new byte[]{
       0x1f, (byte) 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
       0x1f, (byte) 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
 
 
   // The trailer will be overwritten based on crc and output size.
   // The trailer will be overwritten based on crc and output size.
-  private static final byte[] GZIP_TRAILER = new byte[]{
+  private final byte[] gzipTrailer = new byte[]{
       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
 
 
-  private static final int GZIP_HEADER_LEN = GZIP_HEADER.length;
-  private static final int GZIP_TRAILER_LEN = GZIP_TRAILER.length;
+  private final int gzipHeaderLen = gzipHeader.length;
+  private final int gzipTrailerLen = gzipTrailer.length;
 
 
   private Deflater deflater;
   private Deflater deflater;
 
 
@@ -210,12 +210,12 @@ public class BuiltInGzipCompressor implements Compressor {
       return 0;
       return 0;
     }
     }
 
 
-    int n = Math.min(len, GZIP_HEADER_LEN - headerOff);
-    System.arraycopy(GZIP_HEADER, headerOff, b, off, n);
+    int n = Math.min(len, gzipHeaderLen - headerOff);
+    System.arraycopy(gzipHeader, headerOff, b, off, n);
     headerOff += n;
     headerOff += n;
 
 
     // Completes header output.
     // Completes header output.
-    if (headerOff == GZIP_HEADER_LEN) {
+    if (headerOff == gzipHeaderLen) {
       state = BuiltInGzipDecompressor.GzipStateLabel.INFLATE_STREAM;
       state = BuiltInGzipDecompressor.GzipStateLabel.INFLATE_STREAM;
     }
     }
 
 
@@ -225,15 +225,15 @@ public class BuiltInGzipCompressor implements Compressor {
   private void fillTrailer() {
   private void fillTrailer() {
     if (state == BuiltInGzipDecompressor.GzipStateLabel.TRAILER_CRC) {
     if (state == BuiltInGzipDecompressor.GzipStateLabel.TRAILER_CRC) {
       int streamCrc = (int) crc.getValue();
       int streamCrc = (int) crc.getValue();
-      GZIP_TRAILER[0] = (byte) (streamCrc & 0x000000ff);
-      GZIP_TRAILER[1] = (byte) ((streamCrc & 0x0000ff00) >> 8);
-      GZIP_TRAILER[2] = (byte) ((streamCrc & 0x00ff0000) >> 16);
-      GZIP_TRAILER[3] = (byte) ((streamCrc & 0xff000000) >> 24);
+      gzipTrailer[0] = (byte) (streamCrc & 0x000000ff);
+      gzipTrailer[1] = (byte) ((streamCrc & 0x0000ff00) >> 8);
+      gzipTrailer[2] = (byte) ((streamCrc & 0x00ff0000) >> 16);
+      gzipTrailer[3] = (byte) ((streamCrc & 0xff000000) >> 24);
 
 
-      GZIP_TRAILER[4] = (byte) (accuBufLen & 0x000000ff);
-      GZIP_TRAILER[5] = (byte) ((accuBufLen & 0x0000ff00) >> 8);
-      GZIP_TRAILER[6] = (byte) ((accuBufLen & 0x00ff0000) >> 16);
-      GZIP_TRAILER[7] = (byte) ((accuBufLen & 0xff000000) >> 24);
+      gzipTrailer[4] = (byte) (accuBufLen & 0x000000ff);
+      gzipTrailer[5] = (byte) ((accuBufLen & 0x0000ff00) >> 8);
+      gzipTrailer[6] = (byte) ((accuBufLen & 0x00ff0000) >> 16);
+      gzipTrailer[7] = (byte) ((accuBufLen & 0xff000000) >> 24);
 
 
       crc.reset();
       crc.reset();
       accuBufLen = 0;
       accuBufLen = 0;
@@ -245,11 +245,11 @@ public class BuiltInGzipCompressor implements Compressor {
       return 0;
       return 0;
     }
     }
 
 
-    int n = Math.min(len, GZIP_TRAILER_LEN - trailerOff);
-    System.arraycopy(GZIP_TRAILER, trailerOff, b, off, n);
+    int n = Math.min(len, gzipTrailerLen - trailerOff);
+    System.arraycopy(gzipTrailer, trailerOff, b, off, n);
     trailerOff += n;
     trailerOff += n;
 
 
-    if (trailerOff == GZIP_TRAILER_LEN) {
+    if (trailerOff == gzipTrailerLen) {
       state = BuiltInGzipDecompressor.GzipStateLabel.FINISHED;
       state = BuiltInGzipDecompressor.GzipStateLabel.FINISHED;
       currentBufLen = 0;
       currentBufLen = 0;
       headerOff = 0;
       headerOff = 0;

+ 2 - 2
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodec.java

@@ -109,7 +109,7 @@ public class TestCodec {
     }
     }
     // without hadoop-native installed.
     // without hadoop-native installed.
     ZlibFactory.setNativeZlibLoaded(false);
     ZlibFactory.setNativeZlibLoaded(false);
-    assumeTrue(ZlibFactory.isNativeZlibLoaded(conf));
+    assertFalse(ZlibFactory.isNativeZlibLoaded(conf));
     codecTest(conf, seed, 0, "org.apache.hadoop.io.compress.GzipCodec");
     codecTest(conf, seed, 0, "org.apache.hadoop.io.compress.GzipCodec");
     codecTest(conf, seed, count, "org.apache.hadoop.io.compress.GzipCodec");
     codecTest(conf, seed, count, "org.apache.hadoop.io.compress.GzipCodec");
   }
   }
@@ -570,7 +570,7 @@ public class TestCodec {
     }
     }
     // without hadoop-native installed.
     // without hadoop-native installed.
     ZlibFactory.setNativeZlibLoaded(false);
     ZlibFactory.setNativeZlibLoaded(false);
-    assumeTrue(ZlibFactory.isNativeZlibLoaded(conf));
+    assertFalse(ZlibFactory.isNativeZlibLoaded(conf));
     sequenceFileCodecTest(conf, 100, "org.apache.hadoop.io.compress.GzipCodec", 5);
     sequenceFileCodecTest(conf, 100, "org.apache.hadoop.io.compress.GzipCodec", 5);
     sequenceFileCodecTest(conf, 100, "org.apache.hadoop.io.compress.GzipCodec", 100);
     sequenceFileCodecTest(conf, 100, "org.apache.hadoop.io.compress.GzipCodec", 100);
     sequenceFileCodecTest(conf, 200000, "org.apache.hadoop.io.compress.GzipCodec", 1000000);
     sequenceFileCodecTest(conf, 200000, "org.apache.hadoop.io.compress.GzipCodec", 1000000);