Skip to content
This repository has been archived by the owner on Oct 4, 2024. It is now read-only.

comment parser regexp is buggy #7

Open
nlac opened this issue Nov 25, 2011 · 7 comments
Open

comment parser regexp is buggy #7

nlac opened this issue Nov 25, 2011 · 7 comments

Comments

@nlac
Copy link

nlac commented Nov 25, 2011

Hello,

when a long css comment /* ... */ is being parsed, the match() method will fail and the http connection will also fail with status "Aborted". It is invoked in Parser::parseComment().

The right regexp for parsing css comments in php is eg.: #/*._?_/#Us

@agar
Copy link
Owner

agar commented Nov 28, 2011

Could you please send through an example of both the code less you're trying to parse and the PHP code to do so. Ideally as a new unit test. Thanks.

@nlac
Copy link
Author

nlac commented Nov 28, 2011

Hi,find the attached (or download from here http://nlacsoft.net/_test/test_comment.zip).

It is the latest version of the less.php package, i added 1 new

files:/test/test_comment.phpThe script tries to compile

"reset.less" from your test cases. If i call this script from a browser

running a local apache, i get a status "Aborted" and a message "The connection

was reset". If i remove the comment or a part of the comment content from

the beginning of the file, the compilation will be successful.I use

WindowsXP SP3, latest EasyPHP with the default configuration, i tried on two

machines also.RegardsLaszlo~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~

~From:

reply@reply.github.comTo:

nlacsoft@gmail.comDate: 6:18:09 AM, 11.28.2011Subject: Re: [less.php]

comment parser regexp is buggy (#7)~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~

~>>Could you please send through an example of both the

code less you're trying to parse and the PHP code to do so. Ideally as a new

unit test. Thanks.>> >> --- >> Reply to this email

directly or view it on GitHub:>>

https://github.com/agar/less.php/issues/7#issuecomment-2892953>>

@Mordred
Copy link

Mordred commented Nov 30, 2011

I run your "test_comment.php" and it works ok. Try to read your apache error log for more informations why you get "Aborted" status.

@nlac
Copy link
Author

nlac commented Dec 1, 2011

Hi, the apache error log contains nothing.

The issue should be connected to this: http://stackoverflow.com/questions/7620910/regexp-in-preg-match-function-returning-browser-error

I purified the case (find below) and setting pcre.recursion_limit=512 in php.ini i get no error any more but also get no match.
Again.. with this well-known regexp for block comments ( #/*.?/#Us ) it works fine and should be even faster than the pattern below.

//Simplified case for regex bug

//comment from reset.less
$s = '/* Reset.less

  • Props to Eric Meyer (meyerweb.com) for his CSS reset file. We're using an adapted version here that cuts out some of the reset HTML elements we will never need here (i.e., dfn, samp, etc).
  • ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- */';

//regexp from Parser.ph line 308
$matches = array();
preg_match('/^/(?:[^_]|_+[^\/_])_+/\n?/', $s, $matches);

print_r($matches);

@zakjan
Copy link

zakjan commented Dec 5, 2011

Are you working on WIndows? On Linux is everything fine, but on WIndows it fails becouse of Apache is compiled with smaller stack size, and it crashes with stack overflow.

Optimizing the regex helps. /\/\*.*?\*\//s works great. But still, compiling Bootstrap on Windows with Less\Parser tooks very long time for me, 20s on 2GHz. Linux is fine again.

@nlac
Copy link
Author

nlac commented Dec 5, 2011

yes i work on Windows, good to know that on Linux Apache is compiled differently so, that explains the issue. I suggested the same regexp as you above, it implies much less recursion.

@Mordred
Copy link

Mordred commented Jan 15, 2012

Try this: Mordred/less.php@22efc1c

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

No branches or pull requests

4 participants