Backport typelevel#2696 opt-in FHCRC

Co-authored-by: Alessandro Zoffoli <alessandro.zoffoli@moneyfarm.com>
armanbilge · Oct 30, 2021 · d13b5bb · d13b5bb
1 parent 24282e2
commit d13b5bb
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 10 deletions.
diff --git a/build.sbt b/build.sbt
@@ -144,7 +144,20 @@ ThisBuild / mimaBinaryIssueFilters ++= Seq(
   ProblemFilters.exclude[DirectMissingMethodProblem](
     "fs2.io.tls.TLSParameters#DefaultTLSParameters.this"
   ),
-  ProblemFilters.exclude[NewMixinForwarderProblem]("fs2.Stream#LowPrioCompiler.resourceInstance")
+  ProblemFilters.exclude[NewMixinForwarderProblem]("fs2.Stream#LowPrioCompiler.resourceInstance"),
+  ProblemFilters.exclude[ReversedMissingMethodProblem](
+    "fs2.compression#DeflateParams.fhCrcEnabled"
+  ),
+  ProblemFilters.exclude[DirectMissingMethodProblem](
+    "fs2.compression#DeflateParams#DeflateParamsImpl.copy"
+  ),
+  ProblemFilters.exclude[DirectMissingMethodProblem](
+    "fs2.compression#DeflateParams#DeflateParamsImpl.this"
+  ),
+  ProblemFilters.exclude[MissingTypesProblem]("fs2.compression$DeflateParams$DeflateParamsImpl$"),
+  ProblemFilters.exclude[DirectMissingMethodProblem](
+    "fs2.compression#DeflateParams#DeflateParamsImpl.apply"
+  )
 )
 
 lazy val root = project

diff --git a/core/jvm/src/main/scala/fs2/compression.scala b/core/jvm/src/main/scala/fs2/compression.scala
@@ -68,6 +68,13 @@ object compression {
       */
     val flushMode: DeflateParams.FlushMode
 
+    /** A [[Boolean]] indicating whether the `FLG.FHCRC` bit is set. Default is `false`.
+      *  This is provided so that the compressor can be configured to have the CRC16 check enabled.
+      *  Why opt-in and not opt-out? It turned out not all clients implemented that right.
+      *  More context [[https://github.com/http4s/http4s/issues/5417 in this issue]].
+      */
+    val fhCrcEnabled: Boolean
+
     private[compression] val bufferSizeOrMinimum: Int = bufferSize.max(128)
   }
 
@@ -80,14 +87,25 @@ object compression {
         strategy: DeflateParams.Strategy = DeflateParams.Strategy.DEFAULT,
         flushMode: DeflateParams.FlushMode = DeflateParams.FlushMode.DEFAULT
     ): DeflateParams =
-      DeflateParamsImpl(bufferSize, header, level, strategy, flushMode)
+      DeflateParamsImpl(bufferSize, header, level, strategy, flushMode, false)
+
+    def apply(
+        bufferSize: Int,
+        header: ZLibParams.Header,
+        level: DeflateParams.Level,
+        strategy: DeflateParams.Strategy,
+        flushMode: DeflateParams.FlushMode,
+        fhCrcEnabled: Boolean
+    ): DeflateParams =
+      DeflateParamsImpl(bufferSize, header, level, strategy, flushMode, fhCrcEnabled)
 
     private case class DeflateParamsImpl(
         bufferSize: Int,
         header: ZLibParams.Header,
         level: DeflateParams.Level,
         strategy: DeflateParams.Strategy,
-        flushMode: DeflateParams.FlushMode
+        flushMode: DeflateParams.FlushMode,
+        fhCrcEnabled: Boolean
     ) extends DeflateParams
 
     sealed abstract class Level(private[compression] val juzDeflaterLevel: Int)
@@ -582,7 +600,13 @@ object compression {
               }
             ) { case (deflater, _) => SyncF.delay(deflater.end()) }
             .flatMap { case (deflater, crc32) =>
-              _gzip_header(fileName, modificationTime, comment, params.level.juzDeflaterLevel) ++
+              _gzip_header(
+                fileName,
+                modificationTime,
+                comment,
+                params.level.juzDeflaterLevel,
+                params.fhCrcEnabled
+              ) ++
                 _deflate(
                   params,
                   deflater,
@@ -602,7 +626,8 @@ object compression {
       fileName: Option[String],
       modificationTime: Option[Instant],
       comment: Option[String],
-      deflateLevel: Int
+      deflateLevel: Int,
+      fhCrcEnabled: Boolean
   ): Stream[F, Byte] = {
     // See RFC 1952: https://www.ietf.org/rfc/rfc1952.txt
     val secondsSince197001010000: Long =
@@ -611,7 +636,7 @@ object compression {
       gzipMagicFirstByte, // ID1: Identification 1
       gzipMagicSecondByte, // ID2: Identification 2
       gzipCompressionMethod.DEFLATE, // CM: Compression Method
-      (gzipFlag.FHCRC + // FLG: Header CRC
+      ((if (fhCrcEnabled) gzipFlag.FHCRC else zeroByte) + // FLG: Header CRC
         fileName.map(_ => gzipFlag.FNAME).getOrElse(zeroByte) + // FLG: File name
         comment.map(_ => gzipFlag.FCOMMENT).getOrElse(zeroByte)).toByte, // FLG: Comment
       (secondsSince197001010000 & 0xff).toByte, // MTIME: Modification Time
@@ -640,10 +665,14 @@ object compression {
       bytes
     }
     val crc32Value = crc32.getValue
-    val crc16 = Array[Byte](
-      (crc32Value & 0xff).toByte,
-      ((crc32Value >> 8) & 0xff).toByte
-    )
+    val crc16 =
+      if (fhCrcEnabled)
+        Array[Byte](
+          (crc32Value & 0xff).toByte,
+          ((crc32Value >> 8) & 0xff).toByte
+        )
+      else
+        Array.emptyByteArray
     Stream.chunk(moveAsChunkBytes(header)) ++
       fileNameEncoded
         .map(bytes => Stream.chunk(moveAsChunkBytes(bytes)) ++ Stream.emit(zeroByte))

diff --git a/core/jvm/src/test/scala/fs2/CompressionSuite.scala b/core/jvm/src/test/scala/fs2/CompressionSuite.scala
@@ -28,6 +28,8 @@ import java.util.zip._
 
 import cats.effect._
 import fs2.compression._
+import scodec.bits.crc
+import scodec.bits.ByteVector
 
 import org.scalacheck.{Arbitrary, Gen}
 import org.scalacheck.effect.PropF.forAllF
@@ -464,6 +466,35 @@ class CompressionSuite extends Fs2Suite {
       .map(compressed => assert(compressed.length < uncompressed.length))
   }
 
+  test("gzip.compresses input, with FLG.FHCRC set") {
+    Stream
+      .chunk[IO, Byte](Chunk.array(getBytes("Foo")))
+      .through(
+        gzip(
+          fileName = None,
+          modificationTime = None,
+          comment = None,
+          deflateParams = DeflateParams.apply(
+            bufferSize = 1024 * 32,
+            header = ZLibParams.Header.GZIP,
+            level = DeflateParams.Level.DEFAULT,
+            strategy = DeflateParams.Strategy.DEFAULT,
+            flushMode = DeflateParams.FlushMode.DEFAULT,
+            fhCrcEnabled = true
+          )
+        )
+      )
+      .compile
+      .toVector
+      .map { compressed =>
+        val headerBytes = ByteVector(compressed.take(10))
+        val crc32 = crc.crc32(headerBytes.toBitVector).toByteArray
+        val expectedCrc16 = crc32.reverse.take(2).toVector
+        val actualCrc16 = compressed.drop(10).take(2)
+        assertEquals(actualCrc16, expectedCrc16)
+      }
+  }
+
   test("gunzip limit fileName and comment length") {
     val longString: String =
       Array