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

Don't mangle files that start with the gzip magic bytes. #1307

Closed
jmarantz opened this Issue May 12, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@jmarantz
Copy link
Contributor

jmarantz commented May 12, 2016

From mod-pagespeed-discuss thread: https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/mod-pagespeed-discuss/nehDfeYxlTQ/NKWKLLFPCAAJ

A site is using the GD Star Rating plugin, and the CSS for that plugin gets garbled by mod_pagespeed. I suspect this is because the plugin gzips its CSS output but does not give it content-encoding:gzip header. So mod_pagespeed attempts to parse the gzip output as plain CSS and reports lots of encoding warnings.

The output is also broken -- the CSS coming out of mod_pagespeed is not readable.

I think this is the fault of the plugin, but I can't prove it because the download link for the plugin is broken: http://www.gdstarrating.com/index.html . The customer is using GD Star 1.9.22.

As far as I can tell from that site, that whole gd-star system has not had an update in 5 years. However, the problem is avoided by telling mod_pagespeed not to touch it:
ModPagespeedDisallow */gdsr.css.php*

@DoctorLai

This comment has been minimized.

Copy link

DoctorLai commented May 12, 2016

Thanks for your efforts! I have zipped the plugin folder and put it here: https://helloacm.com/data/gd-star-rating.zip

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jun 27, 2016

A gzipped resource should start with the magic bytes 1f8d:

$ curl -sS -H 'Accept-Encoding: gzip' www.google.com | head -c 2 | xxd
00000000: 1f8b

We should be aware of this, and avoid touching anything that starts with that. Maybe we should be like browsers and treat it as telling us that we're processing gzipped data if it then gunzips without errors, but we definitely shouldn't proceed as if it's text.

@jeffkaufman jeffkaufman changed the title CSS file is garbled, Unicode value ... is not valid for interchange when using GD Star Rating plugin Don't mangle files that start with the gzip magic bytes. Jun 27, 2016

@jeffkaufman jeffkaufman self-assigned this Jun 28, 2016

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jun 30, 2016

Looking at the plugin, gdsr-config.php is an ordinary-looking php file, and nothing in the plugin mentions compression. So this is more likely to be a webserver misconfiguration than a problem with the plugin.

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jun 30, 2016

I'm not able to reproduce combining or mangling with gzipped data, just inlining it, so I'm going to send out a CL that just blocks the inlining.

jeffkaufman added a commit that referenced this issue Jul 6, 2016

inliners: don't inline gzipped text
If what we're going to inline into CSS or JS starts with the gzip magic
bytes (file signature) then it's very likely to be gzipped and very
unlikely to be valid CSS or JS, so we should abort the inlining.

Fixes the amount of #1307
that I could repro.  If we later see mangling files in-place (or combining)
then we can extend this.
@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jul 13, 2016

Fixed by 703060b

crowell added a commit that referenced this issue Jul 13, 2016

inliners: don't inline gzipped text
If what we're going to inline into CSS or JS starts with the gzip magic
bytes (file signature) then it's very likely to be gzipped and very
unlikely to be valid CSS or JS, so we should abort the inlining.

Fixes the amount of #1307
that I could repro.  If we later see mangling files in-place (or combining)
then we can extend this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment