From 6258ee01baf3e8b21446775ce4648fe6dedb1f67 Mon Sep 17 00:00:00 2001 From: Alessandro Zoffoli Date: Fri, 29 Oct 2021 12:13:11 +0200 Subject: [PATCH 1/5] Update the default bit in gzip compression to false --- .../fs2/compression/CompressionPlatform.scala | 22 +++++-- .../test/scala/fs2/JvmCompressionSuite.scala | 65 +++++++++++++++++++ .../scala/fs2/compression/DeflateParams.scala | 22 ++++++- 3 files changed, 100 insertions(+), 9 deletions(-) diff --git a/core/jvm/src/main/scala/fs2/compression/CompressionPlatform.scala b/core/jvm/src/main/scala/fs2/compression/CompressionPlatform.scala index abce2d4c78..f0455f01c2 100644 --- a/core/jvm/src/main/scala/fs2/compression/CompressionPlatform.scala +++ b/core/jvm/src/main/scala/fs2/compression/CompressionPlatform.scala @@ -404,7 +404,8 @@ private[compression] trait CompressionCompanionPlatform { fileName, modificationTime, comment, - params.level.juzDeflaterLevel + params.level.juzDeflaterLevel, + params.fhCrcEnabled ) ++ _deflate( params, @@ -425,7 +426,8 @@ private[compression] trait CompressionCompanionPlatform { 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 = @@ -434,7 +436,7 @@ private[compression] trait CompressionCompanionPlatform { 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 @@ -463,10 +465,16 @@ private[compression] trait CompressionCompanionPlatform { 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/JvmCompressionSuite.scala b/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala index 29c437a675..84f54116c7 100644 --- a/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala +++ b/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala @@ -246,6 +246,71 @@ class JvmCompressionSuite extends CompressionSuite { .map(compressed => assert(compressed.length < uncompressed.length)) } + test("gzip.compresses input, with FLG.FHCRC set") { + val uncompressed: Array[Byte] = getBytes("Foo") + val crc16: (Byte, (Byte, Byte)) = { // precomputing for all OSs so that we can have a green run regardless of the OS running the test + val os = System.getProperty("os.name").toLowerCase() + if ( + os.name.indexOf("nux") > 0 || os.name.indexOf("nix") > 0 || os.indexOf("aix") >= 0 + ) // UNIX + (3.toByte, (-89.toByte, 119.toByte)) + else if (os.indexOf("win") >= 0) // NTFS_FILESYSTEM + (11.toByte, (-107.toByte, -1.toByte)) + else if (os.indexOf("mac") >= 0) // MACINTOSH + (7.toByte, (-66.toByte, -77.toByte)) + else // UNKNOWN + (255.toByte, (-112.toByte, -55.toByte)) + } + + val expected = Vector[Byte]( + 31, // magic number (2B) + -117, + 8, // CM + 2, // FLG.FHCRC + 0, // MTIME (4B) + 0, + 0, + 0, + 0, // XFL + crc16._1, // OS + crc16._2._1, // // CRC16 (2B) + crc16._2._2, + 115, // compressed blocks + -53, + -49, + 7, + 0, + -63, // CRC32 (4B) + 35, + 62, + -76, + 3, // ISIZE (4B) + 0, + 0, + 0 + ) + Stream + .chunk[IO, Byte](Chunk.array(uncompressed)) + .through( + Compression[IO].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 => assertEquals(compressed, expected)) + } + test("gunzip limit fileName and comment length") { val longString: String = Array diff --git a/core/shared/src/main/scala/fs2/compression/DeflateParams.scala b/core/shared/src/main/scala/fs2/compression/DeflateParams.scala index ad6a64d63d..e62aea7048 100644 --- a/core/shared/src/main/scala/fs2/compression/DeflateParams.scala +++ b/core/shared/src/main/scala/fs2/compression/DeflateParams.scala @@ -45,6 +45,13 @@ sealed trait DeflateParams { */ val flushMode: DeflateParams.FlushMode + /** A [[Boolean]] indicating whether the `FLG.FHCRC` bit is set. Default is `false`. + * This is provided so that the client can opt-in and enable the CRC16 check for the gzip header. + * 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[fs2] val bufferSizeOrMinimum: Int = bufferSize.max(128) } @@ -57,14 +64,25 @@ object DeflateParams { 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[fs2] val juzDeflaterLevel: Int) From 5d55a6b596ddd48c06533d9f1fa850392196d23f Mon Sep 17 00:00:00 2001 From: Alessandro Zoffoli Date: Fri, 29 Oct 2021 20:41:45 +0200 Subject: [PATCH 2/5] Update the test to use crc function from scodec --- .../test/scala/fs2/JvmCompressionSuite.scala | 54 ++++--------------- 1 file changed, 10 insertions(+), 44 deletions(-) diff --git a/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala b/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala index 84f54116c7..ed6a6fc059 100644 --- a/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala +++ b/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala @@ -30,6 +30,8 @@ import java.nio.charset.StandardCharsets import java.time.Instant import java.util.zip._ import scala.collection.mutable +import scodec.bits.crc +import scodec.bits.ByteVector class JvmCompressionSuite extends CompressionSuite { @@ -247,50 +249,8 @@ class JvmCompressionSuite extends CompressionSuite { } test("gzip.compresses input, with FLG.FHCRC set") { - val uncompressed: Array[Byte] = getBytes("Foo") - val crc16: (Byte, (Byte, Byte)) = { // precomputing for all OSs so that we can have a green run regardless of the OS running the test - val os = System.getProperty("os.name").toLowerCase() - if ( - os.name.indexOf("nux") > 0 || os.name.indexOf("nix") > 0 || os.indexOf("aix") >= 0 - ) // UNIX - (3.toByte, (-89.toByte, 119.toByte)) - else if (os.indexOf("win") >= 0) // NTFS_FILESYSTEM - (11.toByte, (-107.toByte, -1.toByte)) - else if (os.indexOf("mac") >= 0) // MACINTOSH - (7.toByte, (-66.toByte, -77.toByte)) - else // UNKNOWN - (255.toByte, (-112.toByte, -55.toByte)) - } - - val expected = Vector[Byte]( - 31, // magic number (2B) - -117, - 8, // CM - 2, // FLG.FHCRC - 0, // MTIME (4B) - 0, - 0, - 0, - 0, // XFL - crc16._1, // OS - crc16._2._1, // // CRC16 (2B) - crc16._2._2, - 115, // compressed blocks - -53, - -49, - 7, - 0, - -63, // CRC32 (4B) - 35, - 62, - -76, - 3, // ISIZE (4B) - 0, - 0, - 0 - ) Stream - .chunk[IO, Byte](Chunk.array(uncompressed)) + .chunk[IO, Byte](Chunk.array(getBytes("Foo"))) .through( Compression[IO].gzip( fileName = None, @@ -308,7 +268,13 @@ class JvmCompressionSuite extends CompressionSuite { ) .compile .toVector - .map(compressed => assertEquals(compressed, expected)) + .map { compressed => + val headerBytes = ByteVector(compressed.take(10)) + val crc32 = crc.crc32(headerBytes.toBitVector).toByteArray + val expectedCrc16 = crc32.drop(2).take(2).reverse.toVector + val actualCrc16 = compressed.drop(10).take(2) + assertEquals(actualCrc16, expectedCrc16) + } } test("gunzip limit fileName and comment length") { From 5a2c09876bab7802b49cdd373f0d3c6b79cbbc1f Mon Sep 17 00:00:00 2001 From: Alessandro Zoffoli Date: Fri, 29 Oct 2021 20:42:36 +0200 Subject: [PATCH 3/5] Update the wording for fhCrcEnabled parameter --- core/shared/src/main/scala/fs2/compression/DeflateParams.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/shared/src/main/scala/fs2/compression/DeflateParams.scala b/core/shared/src/main/scala/fs2/compression/DeflateParams.scala index e62aea7048..32c966bc29 100644 --- a/core/shared/src/main/scala/fs2/compression/DeflateParams.scala +++ b/core/shared/src/main/scala/fs2/compression/DeflateParams.scala @@ -46,7 +46,7 @@ sealed trait DeflateParams { val flushMode: DeflateParams.FlushMode /** A [[Boolean]] indicating whether the `FLG.FHCRC` bit is set. Default is `false`. - * This is provided so that the client can opt-in and enable the CRC16 check for the gzip header. + * 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]]. */ From 3ea281b9d19a2f6c2adf026a5a5bb73f48421643 Mon Sep 17 00:00:00 2001 From: Alessandro Zoffoli Date: Fri, 29 Oct 2021 20:54:15 +0200 Subject: [PATCH 4/5] Less confusing crc16 test computation --- core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala b/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala index ed6a6fc059..7fa9b2eabc 100644 --- a/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala +++ b/core/jvm/src/test/scala/fs2/JvmCompressionSuite.scala @@ -271,7 +271,7 @@ class JvmCompressionSuite extends CompressionSuite { .map { compressed => val headerBytes = ByteVector(compressed.take(10)) val crc32 = crc.crc32(headerBytes.toBitVector).toByteArray - val expectedCrc16 = crc32.drop(2).take(2).reverse.toVector + val expectedCrc16 = crc32.reverse.take(2).toVector val actualCrc16 = compressed.drop(10).take(2) assertEquals(actualCrc16, expectedCrc16) } From 6b6da69e0a568b9e7ebd72d4bf3574b4c9b77fb4 Mon Sep 17 00:00:00 2001 From: Alessandro Zoffoli Date: Fri, 29 Oct 2021 20:54:30 +0200 Subject: [PATCH 5/5] Update mima filter --- build.sbt | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index f1b9ae82c2..1aa8f5f7bd 100644 --- a/build.sbt +++ b/build.sbt @@ -153,7 +153,20 @@ ThisBuild / mimaBinaryIssueFilters ++= Seq( ProblemFilters.exclude[MissingClassProblem]("fs2.Compiler$TargetLowPriority$MonadCancelTarget"), ProblemFilters.exclude[MissingClassProblem]("fs2.Compiler$TargetLowPriority$MonadErrorTarget"), ProblemFilters.exclude[MissingTypesProblem]("fs2.Compiler$TargetLowPriority$SyncTarget"), - ProblemFilters.exclude[MissingClassProblem]("fs2.Chunk$VectorChunk") + ProblemFilters.exclude[MissingClassProblem]("fs2.Chunk$VectorChunk"), + 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