workers-sdk icon indicating copy to clipboard operation
workers-sdk copied to clipboard

fix: streamed response bodies in Miniflare should not infer non-streaming compression by default

Open petebacondarwin opened this issue 5 months ago • 3 comments

Fixes #8004 Fixes #6577

When serving responses that have not defined content-encoding explicitly, Miniflare was attempting to infer a compression algorithm from the accept-encoding request header. This resulted in streamed responses being compressed with an algorithm (usually gzip) that does not support streaming; so the entire response was buffered in memory and not streamed to the client.

Now Miniflare will only try to infer the content-encoding response from the accept-encoding request header if the inferContentEncoding option is set to true - the default is false.


  • Tests
    • [ ] TODO (before merge)
    • [x] Tests included
    • [ ] Tests not necessary because:
  • Wrangler / Vite E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • [ ] I don't know
    • [x] Required
    • [ ] Not required because:
  • Public documentation
    • [x] TODO (before merge)
    • [ ] Cloudflare docs PR(s):
    • [ ] Documentation not necessary because:
  • Wrangler V3 Backport
    • [x] TODO (before merge)
    • [ ] Wrangler PR:
    • [ ] Not necessary because:

petebacondarwin avatar Jun 17 '25 12:06 petebacondarwin

🦋 Changeset detected

Latest commit: 2d22520afab912f93f149013aeed3f7551b402a4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
miniflare Patch
@cloudflare/pages-shared Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-pool-workers Patch
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Jun 17 '25 12:06 changeset-bot[bot]

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@9625
@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@9625
miniflare

npm i https://pkg.pr.new/miniflare@9625
@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@9625
@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@9625
@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@9625
@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@9625
@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@9625
wrangler

npm i https://pkg.pr.new/wrangler@9625

commit: 2d22520

pkg-pr-new[bot] avatar Jun 17 '25 12:06 pkg-pr-new[bot]

A few notes:

When I tested with workerd br was not buffering/compression - figured out it comes from the compat date 2024-04-29

One thing that I don't get is how we come with the desiredEncoding - that is the encoding of the response we will return to the client.

desiredEncoding is set to the the first listed accepted encoding with the highest weight. I don't understand why?

i.e. if the original request is accept-encoding: gzip, br and the worker returns a content-type: br then we will change the encoding to gzip in ensureAcceptableEncoding while br should be ok.

Is this done to match FL?

Maybe we can change that for at least Transfer-Encoding: chunked to solve the streaming issue without the need to add a flag. We could return the response without changes when its encoding is compatible with the accept-encoding (instead have matching desiredEncoding).

One problem with the inferContentEncoding = false flag is that it might generate invalid responses (i.e. the client request gzip, *;q=0 and the response is not encoded)

vicb avatar Jun 18 '25 06:06 vicb

About the PR title: "streamed response bodies in Miniflare should not infer non-streaming compression by default"

IIUC we do infer the actual encoding from the accepted encoding - that's expected. But the PR do prefer identity encoding when possible over compression encoding.

Also I don't think the current changes are specific to "streamed responses"?

vicb avatar Jun 23 '25 11:06 vicb