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

Enable brotli decompression if it is available #620

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

immerrr
Copy link
Contributor

@immerrr immerrr commented Nov 12, 2021

This PR enables brotli decompression of responses if it is available (via brotli, brotlipy or brotlicffi)

I have added all three brotli variations to the test suite, but I'm at a bit of a loss, because httpbin brings in brotlipy unconditionally, so there is no real way to test:

  • how vcrpy will behave without any brotli implementation
  • how vcrpy will behave with a different brotli implementation (in theory, all three support the same brotli.decompress(data) API)

Happy to hear any feedback, but also feel free to make any adjustments that feel right (I have enabled edits from maintainers).

Fixes #599

@immerrr immerrr force-pushed the enable-decompressing-brotli-encoded-responses branch from 9d94e14 to c7b3b93 Compare November 12, 2021 16:46
@immerrr immerrr force-pushed the enable-decompressing-brotli-encoded-responses branch 2 times, most recently from 7fbcf70 to 3e31acf Compare August 10, 2022 08:23
@immerrr immerrr force-pushed the enable-decompressing-brotli-encoded-responses branch from 3e31acf to 36d72ea Compare August 10, 2022 09:06
@immerrr
Copy link
Contributor Author

immerrr commented Aug 10, 2022

@jairhenrique would you be able to look at this at your convenience?

Copy link
Contributor

@CharString CharString left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compression is an ongoing field of research. This suggestion will make it easier to adopt new algorithms.

AVAILABLE_DECOMPRESSORS = {"gzip", "deflate"}
if brotli is not None:
AVAILABLE_DECOMPRESSORS.add("br")

Copy link
Contributor

@CharString CharString Oct 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is easier to expand with future or custom decompressors if AVAILABLE_DECOMPRESSORS is a Dict[[str], Callable[[bytes], str]]

AVAILABLE_DECOMPRESSORS = {
    "brotli": brotli.decompress,
    "deflate": zlib.decompress,
    "gzip": lambda body: zlib.decompress(body, zlib.MAX_WBITS | 16),
}

Then decompress_body can just be

def decompress_body(body, encoding):
    return AVAILABLE_DECOMPRESSORS[encoding](body)

Adding a new scheme will be as easy as adding a function to the dict.

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.

support "conent-encoding: br"
3 participants