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

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

Open
ggramaize opened this issue Aug 31, 2018 · 8 comments

Comments

@ggramaize
Copy link
Contributor

ggramaize commented Aug 31, 2018

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
Copy link
Contributor Author

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
Copy link
Owner

barnacs commented Aug 31, 2018 via email

@ggramaize
Copy link
Contributor Author

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
Copy link
Owner

barnacs commented Aug 31, 2018 via email

@ggramaize
Copy link
Contributor Author

ggramaize commented Aug 31, 2018

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
Copy link
Collaborator

gaul commented Aug 31, 2018

Could you add a test to compy_test.go?

@ggramaize
Copy link
Contributor Author

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
Copy link
Contributor Author

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

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

No branches or pull requests

3 participants