search

zendframework / zend-filter

Filter component from Zend Framework
BSD 3-Clause "New" or "Revised" License
68 stars 36 forks source link

issue #88: Prevent infinite looping on empty/short HTML comment #89

Closed TotalWipeOut closed 5 years ago

TotalWipeOut commented 5 years ago

This fixes issue detailed in #88

TotalWipeOut commented 5 years ago

Thanks for taking a look - Great, leave with me, I will add that to the unit tests and get it working

TotalWipeOut commented 5 years ago

@TotalWipeOut Thanks for your contribution.

After your changes the following example is not working correctly:

A <!--My favorite operators are > and <!--> B

After filtering we should have just A B, but with your changes we are getting A only.

This is from: https://www.w3.org/TR/html52/syntax.html#comments

@webimpress Had a look at this, I would suggest this a different bug to this one. The way the code is working is that it strips comments starting from the end of the string, so it finds <!--> first, and strips it. Then in the remainder we have only <!-- without an -->, which the code is designed to strip to the end of the string - which it does. Fixing this is almost a refactor. Also, interesting is that running this html snippet on master branch causes and infinite loop - so this particular check has never been run! I would prefer this issue only fix the infinite loop issue.

UPDATE: @webimpress OK, so I re-wrote the loop that strips HTML comments. Now passing all unit tests including your suggestion. and now no need for a regex...

michalbundyra commented 5 years ago

@TotalWipeOut It looks good to me, thanks! 👍

@icanhazstring would you mind to have another look, please?

michalbundyra commented 5 years ago

Thanks, @TotalWipeOut!