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

"Content-Encoding: none" breaks mod_deflate #548

Closed
philfry opened this issue Dec 21, 2015 · 19 comments
Closed

"Content-Encoding: none" breaks mod_deflate #548

philfry opened this issue Dec 21, 2015 · 19 comments
Assignees

Comments

@philfry
Copy link

philfry commented Dec 21, 2015

Hi,

I'm using mod_deflate instead of grav's builtin gzip-compression because it's more flexible for my needs.

As grav sets the header Content-Encoding: none in system/src/Grav/Common/Grav.php when having system.cache.gzip set to false, grav is preventing httpd from compressing the output. Imho there's no need for setting this header when compression is not wanted.

Kind regards,

Phil

@rhukster
Copy link
Member

I agree, will fix.

@rhukster
Copy link
Member

@mahagr looks like your shutdown fix added a Content-Encoding: none, is this required for your fix to function on your end? d2660e0

@mahagr
Copy link
Member

mahagr commented Dec 22, 2015

Actually that Content-Encoding: none is indeed needed to make the functionality to work. Otherwise connection will not be closed with mod_deflate enabled because of the Content-Length is set (but is too long after the compression).

Basically the whole fix was Content-Encoding: none; the another change to force ob_start() was just something I found to be buggy if the browser didn't support compressing.

Please do not remove the line; it just breaks the code.

@mahagr
Copy link
Member

mahagr commented Dec 22, 2015

PHP does not have a way to close STDOUT. If you have something to process after the page has been sent, browser (and mod_deflate) will keep expecting more data and user needs to wait until PHP stops running before the page has been fully loaded. The only way to work around this is to send Content-Length and make client to close the connection when it has received all the data (except if you are using FastCGI, which has command to close the connection).

Assuming that you aren't using FastCGI, mod_deflate will never get the information that all the data has been sent and it will continue to expect more data regardless what you try to do. So the only way is not to use mod_deflate in this case.

All of this said:

  • It is not possible to use mod_deflate unless you are using FastCGI
  • It is possible to add special code path for FastGCI + mod_deflate which doesn't use gzhandler

In the second case request should probably not send Content-Length and connection close.

@mahagr
Copy link
Member

mahagr commented Dec 22, 2015

New logic should allow mod_deflate to do the hard work, but only if you are using FastCGI. You need to disable Grav's builtin gzip handler to use mod_deflate.

Without FastCGI there is not much that can be done because of the limitations of PHP. In this case you are better off using Grav's implementation of gzip.

@mahagr mahagr closed this as completed Dec 22, 2015
@philfry
Copy link
Author

philfry commented Dec 22, 2015

Hi, thanks for your feedback.

is Content-Encoding: none even valid (rfc2616)? I've never seen it in any other applications before.
What about using flush(), ob_end_flush(), session_write_close() and, for fcgi, fastcgi_finish_request()? That should work fine even without the Content-Encoding header set to none.

fwiw, I'm using php-fpm. It might be possible workaround it using mod_headers, but that cannot be the final solution :)

@mahagr
Copy link
Member

mahagr commented Dec 22, 2015

We replied at the same time. Please see my comment from above.

@mahagr
Copy link
Member

mahagr commented Dec 22, 2015

To reply to your questions, I fixed the code to close the connection properly for you (php-fpm).

Content-Encoding: none is used in all the examples on how to prevent mod_deflate from compressing the PHP output. I don't think there is a better workaround for this -- I guess it is better to send slightly broken response than to let user to wait seconds or even minutes to get any response..

If you have ideas on how to do this in a better way, I'm all ears. At least the issue doesn't exist anymore with FastCGI. :)

@philfry
Copy link
Author

philfry commented Dec 22, 2015

great, thanks a lot 👍

@Monarobase
Copy link

Hello,

This trick, seems to break on LiteSpeed when gzip is disabled in grav. Litespeed manages it's own gzip compression using a gzip cache which is much more efficient than what PHP can do) and add's gzip to the headers if it doesn't exist resulting in :

Content-Encoding:none,gzip

The result of this is that the browser show's the Gzip compressed content instead of compressing it.

If we comment out Content-Encoding:none the page loads without any delay on litespeed, I've also asked them if they have an equivilent to fastcgi_finish_request().

Some of your featured hosts also use litespeed, so I guess they also have to deal with this issue too.

@mahagr
Copy link
Member

mahagr commented Jun 30, 2016

Unless they have similar function to fastcgi_finish_request(), gzip from the server cannot really used. If they do not have the function, ask what is the correct way to stop server from compressing the output.

The main reason for the code we have is that it allows us to close the connection and to continue processing some cron job taking a lot time. Without the ability to close the connection, users would see page loading a long time even if they already received the whole page.

@litespeedtech
Copy link

LiteSpeed can compress response with content-length header set without any problem. A properly written web server should be able handle it. When LiteSpeed compress the response body, it replaces "Content-Length" header with "Transfer-Encoding: chunked", the compressed response wont cause browser to spin forever.

Looks like we need to drop the "Content-Encoding:none" header internally.

@ulab
Copy link

ulab commented Dec 3, 2016

I just found this issue because I was having a problem validating my site. The W3C Markup Validation Service will not work with a "Content-Encoding: none" header. It stops with an error "Don't know how to decode Content-Encoding 'none'".

Changing the value for system.cache.gzip to true in user/config/system.yaml fixed that.

@mahagr
Copy link
Member

mahagr commented Dec 5, 2016

@rhukster I think the correct one would be using Content-Encoding: identity, which means "no compression, no modification"

@miljan-aleksic
Copy link

What is the state of this issue? I have run on it as @ulab and his solution works fine, but that means the default behaviour is buggy?

@mahagr
Copy link
Member

mahagr commented Jan 5, 2017

Try latest CI build of Grav if it works in there.

@rhukster
Copy link
Member

Ok, so i've made a change to this. Changing to 'identity' was causing some encoding problems for some people. I've rolled back the default to 'none' which actually works more reliably with our onShutDown() event. However, you can set the cache: allow_webserver_gzip: true in system.yaml if you want to change the behavior and don't care about the onShutDown() potential slowdown.

@miljan-aleksic
Copy link

@rhukster, you may want to comment this behaviour in the docs. The impression I got when tried to validate my site with W3C validator is that there is something broken. Gzip is recommended, but now should be considered a must if you want your site to be correctly validated and/or perhaps indexed (if W3C refuses to read the content, there is no warranty others will do).

@rhukster
Copy link
Member

I have a note in my "items to document" mega issue: getgrav/grav-learn#356

You can enable Gzip via the grav option directly. This will ensure gzip is working for the .php pages, and it will still work with onShutdown() event properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants