From d13b5bb7d53fd639433ecafc819de29e272b541c Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Sat, 30 Oct 2021 16:45:43 +0000 Subject: [PATCH] Backport #2696 opt-in FHCRC Co-authored-by: Alessandro Zoffoli --- build.sbt | 15 +++++- core/jvm/src/main/scala/fs2/compression.scala | 47 +++++++++++++++---- .../src/test/scala/fs2/CompressionSuite.scala | 31 ++++++++++++ 3 files changed, 83 insertions(+), 10 deletions(-) diff --git a/build.sbt b/build.sbt index 5ce26f65cb..7537f1cb7f 100644 --- 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 index 21ed2ff8cf..04cfb20c0b 100644 --- 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 index d77e26f67c..2c0c14264e 100644 --- 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