-
Notifications
You must be signed in to change notification settings - Fork 789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft GUnzip middleware #5546
base: main
Are you sure you want to change the base?
Draft GUnzip middleware #5546
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the slow review. This looks like a good start.
inflateParams: InflateParams | ||
): Request[F] = { | ||
val newBody = req.body.through(Compression[F].gunzip(inflateParams)).flatMap(_.content) | ||
val newHeaders = req.removeHeader[`Content-Encoding`].putHeaders(`Content-Length`(69)).headers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Content-Length
definitely isn't hard coded, and I don't think we can even calculate it in advance of consuming (and inflating) the stream. Removing it is correct.
We could add a Transfer-Encoding: chunked
header, since we usually expect either a Content-Length
or that header. But the request didn't actually come in that way. I could argue that one both ways. Is there anybody who can argue it just one way?
Did something similar a while back. Feel free to pick and choose from this as you like: import cats.data.Kleisli
import cats.effect._
import org.http4s._
import org.http4s.headers.`Content-Encoding`
object GZipRequestMiddleware {
def apply[F[_]: Sync](service: HttpRoutes[F], bufferSize: Int = 100 * 1024): HttpRoutes[F] =
Kleisli { (req: Request[F]) =>
val req2 = req.headers.get[`Content-Encoding`] match {
case Some(header) if satisfiedByGzip(header) =>
val decoded = req.body
.through(fs2.compression.gunzip[F](bufferSize))
.flatMap(_.content)
.handleErrorWith { e =>
throw MalformedMessageBodyFailure(
"Failed to decode gzippped request body",
Some(e)
)
}
req
.removeHeader[`Content-Encoding`]
.withEntity(decoded)(
EntityEncoder.entityBodyEncoder
) // resolving implicit conflict
case _ => req
}
service(req2)
}
private def satisfiedByGzip(header: `Content-Encoding`) =
header.contentCoding.matches(ContentCoding.gzip) || header.contentCoding
.matches(ContentCoding.`x-gzip`)
} import cats.Applicative
import java.io.ByteArrayOutputStream
import java.util.zip.GZIPOutputStream
import cats.effect.IO
import org.http4s.{EntityEncoder, HttpRoutes, Request}
import org.http4s.dsl.io._
import org.http4s.implicits._
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
class GZipRequestMiddlewareSpec extends AnyFlatSpec with Matchers {
private val routes = HttpRoutes.of[IO] {
case req @ POST -> Root / "echo" =>
Ok(req.body)(implicitly(Applicative[IO]), EntityEncoder.entityBodyEncoder)
}
private val app = GZipRequestMiddleware(routes).orNotFound
private val testRequest = Request[IO](uri = uri"/echo", method = POST).withEntity("hello")(EntityEncoder.stringEncoder)
it should "leave unzipped requests alone" in {
val result = app.run(testRequest)
.unsafeRunSync()
.bodyText
.compile
.string
.unsafeRunSync()
result shouldBe "hello"
}
private val compressedHello = {
val out = new ByteArrayOutputStream()
val gzip = new GZIPOutputStream(out)
gzip.write("hello".getBytes())
gzip.close()
out.toByteArray
}
it should "decompress zipped requests" in {
val gzipRequest = testRequest.withEntity(compressedHello)(EntityEncoder.byteArrayEncoder)
.withHeaders(Ok("OK", "Content-Encoding" -> "gzip").unsafeRunSync().headers)
val result = app.run(gzipRequest)
.unsafeRunSync()
.bodyText
.compile
.string
.unsafeRunSync()
result shouldBe "hello"
}
it should "remove content-encoding" in {
val gzipRequest = testRequest.withEntity(compressedHello)(EntityEncoder.byteArrayEncoder)
.withHeaders(Ok("OK", "Content-Encoding" -> "gzip").unsafeRunSync().headers)
// If content-encoding isn't filtered, this will fail as we attempt to decode the
// request body twice.
val appWrappedTwice = GZipRequestMiddleware(GZipRequestMiddleware(routes)).orNotFound
val result = appWrappedTwice.run(gzipRequest)
.unsafeRunSync()
.bodyText
.compile
.string
.unsafeRunSync()
result shouldBe "hello"
}
} |
Draft addressing server side. Client side remains to be implemented. Needs tests.
Closes #4981