-
Notifications
You must be signed in to change notification settings - Fork 638
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
Memory leak issue injecting invalid CSS into view #5912
Comments
As of Craft 3.4, CSS injected into the View is minified automatically when appropriate. It seems like there was similar behavior when unclosed comments were encountered by the CSS minification library: |
@Mosnar That certainly aligns with what I'm seeing... |
It seems like the minification author isn't interested in fixing it. Does Craft itself need to be more defensive against broken CSS code? |
The maintainer claims that the issue was fixed in versions 2.4.8-p10 and 3.0.0. Craft is pulling in 4.0.0+, via That said, it appears that the issue was fixed via tubalmartin/YUI-CSS-compressor-PHP-port@210b058, which really is fixing a bug where changing the PHP memory_limit at runtime wasn’t working correctly. Which suggests that there’s no actual bug with minifying invalid CSS, besides that it tends to take more memory to minify than valid CSS. So @chasegiunta I’m guessing in your case the issue is that PHP isn’t allowed to change its config settings at runtime. Please help verify by changing PHP’s |
@chasegiunta Thanks. Able to reproduce here as well, and just posted an issue about it (tubalmartin/YUI-CSS-compressor-PHP-port#58) as well as a PR that fixes it (tubalmartin/YUI-CSS-compressor-PHP-port#59). |
Thanks @brandonkelly! Is that a similar PR fix to this one? tubalmartin/YUI-CSS-compressor-PHP-port#49 . Based on the author's activity, we may not see anything merged in anytime soon 😬. Would using a fork be an option? |
Oh, missed that. And yeah, the maintainer does seem to be MIA. Just added a guard against unclosed CSS bodies to Craft, which fixes this for the next release. |
Brilliant, thanks @brandonkelly! 🍺 |
Craft 3.4.16 is out now with that change. |
FYI @assetic-php & @wintercms will be switching to https://github.com/wikimedia/minify since that is actively maintained, might be worth looking into. |
Description
Though this memory leak issue has shown up with a particular plugin (CPCSS), it's most likely a deeper issue with Craft CMS and/or Yii, so raising it here.
It's not difficult to recreate. All we need is the CPCSS plugin and some text input into one of it's field settings.
Steps to reproduce
See doublesecretagency/craft-cpcss#12
Additional info
The text was updated successfully, but these errors were encountered: