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

CSS crashing YUI-CSS-compressor-PHP-port #11

Closed
futtta opened this issue Jan 4, 2014 · 6 comments
Closed

CSS crashing YUI-CSS-compressor-PHP-port #11

futtta opened this issue Jan 4, 2014 · 6 comments

Comments

@futtta
Copy link

futtta commented Jan 4, 2014

Hi Túbal;
I'm using your YUI-CSS port in my WordPress plugin (Autoptimize).

A user filed a bugreport that my plugin was generating internal server errors. After investigating the problem is due to YUI-CSS-compressor-PHP-port not being able to optimize specific CSS of All In One Buttons, another WordPress plugin. The YUI-CSS port's GUI on my healthy Linux-VPS doesn't succeed in optimizing that CSS either, so this indeed seems like a bug?

Looking forward to your feedback, let me know if I can help in any way!

frank

@futtta
Copy link
Author

futtta commented Jan 6, 2014

So I've been looking into this a lot more over the last couple of days, assisted by two Autoptmize-users who had similar problems. They were on Xampp by the way, which due to default Apache config on Windows exhibits this problem a lot sooner, as mentioned here by others before as well.

The issue is specifically with line 294, which tries to hide colons before stripping whitespace (297);

$css = preg_replace_callback('/(?:^|\})(?:(?:[^\{\:])+\:)+(?:[^\{]*\{)/', array($this, 'replace_colon'), $css);

When processing long css-strings (my tests worked until 10396 bytes but failed the CSS-string was bigger, this happens for CSS that could not be chunked securely earlier in the code, e.g. when you have a long list of selectors for a declaration block), this line of code causes a stack overflow in PCRE (regex-lib used by PHP) which in turn makes Apache segfault. There are a lot of bug-reports about this disruptive behavior on bugs.php.net, all of which are closed stating this is a problem in PCRE that is also discussed in that bug-db.

Possible solutions in order of (personal) preference;

  1. simplify regex, making it use less space on the stack (example by PCRE-team here)
  2. replace the regex on line 297 (which removes the whitespace) with another -less efficient?- one, removing the need to apply the colon-magic just before and after
  3. check str_len($css) and don't replace colons if lenght > chunck_size (or chunk_size*1.5 or *2)
  4. ask users to increase Apache's threadstacksize
    5. leave as is

I tried both (2), (3) and (4) successfully, but I'm not smart enough to do (1). So who is?

frank

@futtta
Copy link
Author

futtta commented Jan 28, 2014

As not being smart enough is no excuse, I came up with the following alternative approach;

$css = preg_replace_callback('/(?:(?:^|\})([^\{]*\s:))/', array($this, 'replace_colon'), $css);

Based on tests with un-chunkable large CSS-strings this (see here for a visual representation of the regex) works while it does seem to replace the colon where needed and the test-suite indeed reports an identical success rate.

This got released today as part of Autoptimize 1.8.

@tubalmartin
Copy link
Owner

I'll look into this ASAP. Thanks for your work!

@futtta
Copy link
Author

futtta commented Feb 26, 2014

Any news Túbal?

@tubalmartin
Copy link
Owner

I have already fixed this issue. Thank you for your work! Appreciated! I'll release a new version of the lib today.

@tubalmartin
Copy link
Owner

Fixed in v2.4.8-3

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

2 participants