Skip to content

fix(brotli-plugin): Skip brotli compression for upstream compressed response#10740

Merged
juststillthinking merged 12 commits into
apache:masterfrom
SilentEntity:skip_brotli_compression
Jan 12, 2024
Merged

fix(brotli-plugin): Skip brotli compression for upstream compressed response#10740
juststillthinking merged 12 commits into
apache:masterfrom
SilentEntity:skip_brotli_compression

Conversation

@SilentEntity

@SilentEntity SilentEntity commented Jan 2, 2024

Copy link
Copy Markdown
Contributor

Skipping the brotli compression if the upstream response is already compressed.

The Problem

The problem occurs when the upstream service sends compressed data/response either in gzip or deflate. Apisix tries to compress data in brotli which is already compressed and, in turn, returns encoded data that can not be used by the browser or application.

Possible Solutions

There are two options to solve the problem first to skip the further compression and return with the upstream compressed response. The second is to decompress the upstream response and then compress using brotli, which is not viable on apisix gateway, it increases the overhead of the gateway and results in increasing latency and computation requirements.

This PR solves the Issue by skipping the brotli compression if the upstream response is compressed.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@SilentEntity SilentEntity changed the title Skip brotli compression fix(brotli-plugin): Skip brotli compression for upstream compressed response Jan 2, 2024
@SilentEntity
SilentEntity marked this pull request as ready for review January 3, 2024 05:29
Comment thread apisix/plugins/brotli.lua
@juststillthinking

Copy link
Copy Markdown
Contributor

Could you add some test cases?

@juststillthinking

Copy link
Copy Markdown
Contributor

I think it's reasonable, multiple compressions may cause unnecessary trouble

@juststillthinking

Copy link
Copy Markdown
Contributor

please make the ci pass

@juststillthinking juststillthinking added wait for update wait for the author's response in this issue/PR and removed need test cases labels Jan 5, 2024
@SilentEntity

Copy link
Copy Markdown
Contributor Author

okay, it's ready

@SilentEntity

Copy link
Copy Markdown
Contributor Author

The typo is fixed. It will pass in CI.

@juststillthinking

Copy link
Copy Markdown
Contributor

The ci still failed

@SilentEntity

Copy link
Copy Markdown
Contributor Author

Hi @monkeyDluffy6017, please check now

@juststillthinking juststillthinking removed wait for update wait for the author's response in this issue/PR user responded labels Jan 11, 2024
@juststillthinking
juststillthinking merged commit 781e8f6 into apache:master Jan 12, 2024
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.

bug: Skip brotli compression if upstream response is compressed

3 participants