Revert "Defend against brotli decompression bombs (#8227)"

[swankjesse]

This reverts commit 84a028d.

I’ve been giving this change some thought, and I’ve decided that my original fix is a layering violation.

OkHttp’s ResponseBody API streams bytes to the reader, and there’s no reason to think the decompressed data is going to yield a crash. For example, the consumer might be doing something super basic, like playing a looping sound on a speaker.

If the client is going to make a call that could result in an overly-large response, the client should put limits on how many bytes it reads.

[yschimke]

OkHttp’s ResponseBody API streams bytes to the reader, and there’s no reason to think the decompressed data is going to yield a crash. For example, the consumer might be doing something super basic, like playing a looping sound on a speaker.

I am not sure I agree with that.

Even if you are counting the bytes and discarding. It's a bomb. The decoder will read some small number of input bytes and have a huge block of data to write.

Maybe this is truly streamed at some layer in the brotli impl, but can we confirm that. That the brotli library is writing the 10gb in small chunks that we can pause.

Similar advice at google/brotli#960

I guess if my concern is true, this doesn't actually fix it really.

[oldergod]

Does it mean you wanna expose those APIs to configure that? Or just that it's not OkHttp's responsibility to deal with that?

[yschimke]

Brotli isn't enabled by default - so you could make this a required param for adding it?

[swankjesse]

Here’s where I landed after pondering this...

The reported bomb is only destructive if the application layer does something trusting with it; perhaps by loading it all into memory, or by writing it all to a file system. And in these cases I don’t think we need Brotli, any problematic HTTP response could harm the receiver.

Probably the most likely victim is a person writing a web crawler with OkHttp, because that naturally involves making requests to untrusted servers. But I believe the burden is on the application layer to limit how many resources are consumed by each response.

[yschimke]

And in these cases I don’t think we need Brotli, any problematic HTTP response could harm the receiver.

Yep - gzip also https://en.wikipedia.org/wiki/Zip_bomb