barnacs / compy

HTTP/HTTPS compression proxy
ISC License
207 stars 35 forks source link

Minifier plugin triggers a Content encoding error if content is compressed with brotli by the target server #47

Open ggramaize opened 6 years ago

ggramaize commented 6 years ago

Hi,

For a reason unknown to me, compy triggers Content encoding errors on the client when both the browser and the target server support brotli, and when the minifier plugin is enabled.

Known failure cases which triggers Content encoding errors are:

One solution to circumvent the issue is to mask br when passing the Accept-encoding headers, if the minify plugin is enabled.

The more elegant one would be to check why the content isn't encoded back in brotli when processed by the minify plugin.

Kind regards,

ggramaize commented 6 years ago

From what I understand, this looks like a feature ordering issue in the transcoding section.

The issue never happens with pictures, because pictures never get compressed over HTTP, as compressing an already compressed file is pointless, wastes CPU resources, and bandwidth.

The Content Encoding Error never happens with the Identity transcoder, as it stupidly passes the raw data it received, and the zip plugin ensures proper transcoding. The only thing I don't get is why there are no problem when the incoming content is gzip-encoded with the minifier.

barnacs commented 6 years ago

Just had a quick look at the relevant code and I see no attempt to ever decompress brotli received from the server. My guess is that the compressed content is passed to the minifier as is and gets corrupted.

ggramaize commented 6 years ago

Ignore my second post, I didn't dig enough into the code to understand it well.

The zip modules never decompresses brotli. I've just performed tests with this gist, which solved the issue. I need to improve this, by checking the flags logic before creating a PR.

barnacs commented 6 years ago

Just pushed a branch called brotli-dec but unfortunately I don't have a setup avaiable to test any changes at the moment. It's very similar to yours, the SkipCompressed flag is just a kind of optimization to avoid decompressing/recompressing content with the same algo when no other transcoding is required.

ggramaize commented 6 years ago

We might experience similar issues if unsupported encodings are advertised by the client and supported by the target server (e.g. deflate and sdch).

Accept-Encoding filtering is also required to fix this.

Edit: I'll check your branch this evening.

gaul commented 6 years ago

Could you add a test to compy_test.go?

ggramaize commented 6 years ago

That would be difficult to test internally, because:

  1. You need to build a test case with a remote target that serves content with brotli (and therefore https). Visiting https://www.google.com can do the trick, while expecting a non-zero answer and an HTTP/200 code.
  2. It needs a fix for HTTP over CONNECT, which I can't ship yet, because it's incompatible with the --cert option (Implicit TLS on client side).
ggramaize commented 6 years ago

I'm performing tests with mitm + minifier enabled and Firefox 63.0 (nightly), which works fine on the sites I've had issues with.