Skip to content
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

Fix streaming response compression #2798

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

kyri-petrou
Copy link
Collaborator

This PR fixes a bug where if a streaming response has a known size, netty fails to compress it and the response is all botched up. The main culprit is that the ByteBuf objects need to be wrapped in a DefaultHttpResponse prior to writing them to the Netty channel for Netty to be able to compress them properly.

In addition, there is also another minor bug where the content-length from headers is not being extracted correctly, as it seems that netty will mutate the response object after writeAndFlush has been invoked on it, which means we need to extract the content-length before we call it

To reproduce, run the following code and open http://localhost:8080/foo on a browser:

import zio.*
import zio.http.*
import zio.http.Server.Config.ResponseCompressionConfig
import zio.stream.ZStream

object Foo extends ZIOAppDefault {
  val body = Chunk.fromArray("hello world".getBytes)

  val foo =
    Method.GET / "foo" -> handler(
      ZIO.succeed(Response(body = Body.fromStream(ZStream.fromChunk(body), body.length)))
    )

  override val run =
    Server
      .serve(Routes(foo).toHttpApp)
      .provide(
        Server.live,
        ZLayer.succeed(Server.Config.default.port(8080).responseCompression(ResponseCompressionConfig.default)),
      )
}

@jdegoes jdegoes merged commit 343d4f1 into zio:main Apr 24, 2024
25 checks passed
@kyri-petrou kyri-petrou deleted the fix-stream-compression branch April 24, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants