Cannot use empty string in Sanitizer truncate option 'collapseLinesWith'

[Ferdi Derksen]

Hi all!

FYI / to be fixed

Background: I tried updating on of our websites from a PHP7 to a PHP8 version, also migrating from 3.0.200 to 3.0.229 when an existing piece of code triggered a timeout.

In this case I'm using the Sanitizer->truncate (WireTextTools - truncate) method with option 'collapseLinesWith' using an empty string as value. This triggered a timeout.
Finding out why this occurred, I cannot find a clue. It seems between the referred versions there wasn't an update for this method.

Anyhow, the fix is quite simple. Just using a blank space instead of the empty string fixed it.

$html = '<p>Ligula sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Maecenas faucibus mollis interdum. Praesent commodo cursus magna, vel scelerisque nisl consectetur et. Nullam id dolor id nibh ultricies vehicula ut id elit. Donec ullamcorper nulla non metus auctor fringilla.</p>

<h3>Header</h3>

<p>Fusce dapibus, tellus ac cursus commodo, tortor mauris condimentum nibh, ut fermentum massa justo sit amet risus. Donec ullamcorper nulla non metus auctor fringilla. Cras justo odio, dapibus ac facilisis in, egestas eget quam. Donec sed odio dui. Praesent commodo cursus magna, vel scelerisque nisl consectetur et. Aenean eu leo quam. Pellentesque ornare sem lacinia quam venenatis vestibulum.</p>

<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis mollis, est non commodo luctus, nisi erat porttitor ligula, eget lacinia odio sem nec elit. Vestibulum id ligula porta felis euismod semper. Nullam id dolor id nibh ultricies vehicula ut id elit. Aenean lacinia bibendum nulla sed consectetur. <a href="https://www.processwire.com" target="_blank">Look at this!</a>.</p>';

$intro = $sanitizer->truncate($html, [
  'type' => 'word',
  'maxLength' => 350,
  'visible' => false,
  'collapseLinesWith' => '', // BUG! - just a blank space fixed it
  'keepFormatTags' => false,
]);
var_dump($intro);

[poljpocket]

A quick code read gives me this problem. I think you are right, this is a bug!

Line 443 of WireTextTools must be the culprit: https://github.com/processwire/processwire/blob/3cc76cc886a49313b4bfb9a1a904bd88d11b7cb7/wire/core/WireTextTools.php#L443

while(strpos($str, "$r$r") !== false) { ... }

PHP 8
strpos will always return 0 (zero) for an empty string as needle (which is the case for $r = ''). And thus we get an endless loop.

PHP 7
strpos will trigger a warning because the needle is an empty string. Nevertheless, the result is false and the loop breaks. I think your code was generating these on PHP7 but didn't fail and thus, you didn't care about it.

This behaviour matches the updated strpos function in PHP 8, see Changelog here: https://www.php.net/manual/en/function.strpos.php

@Ferdi Derksen this is your finding. Would you like to post the issue? I can produce a PR for it afterwards if @ryan would like me to. The fix is easy enough..

while($r && strpos($str, "$r$r") !== false) { ... }

Edited August 28 by poljpocket

[poljpocket]

Update to my last post. The fix sadly isn't easy enough. At least for the whole function "collapse" it is not. The line itself is fixed but, this code here does unexpected stuff when $r = ''.

https://github.com/processwire/processwire/blob/3cc76cc886a49313b4bfb9a1a904bd88d11b7cb7/wire/core/WireTextTools.php#L454-L456

foreach($this->getPunctuationChars() as $c) {
    if(strpos($str, "$c$r")) $str = str_replace("$c$r", "$c ", $str);
}

Here, the strpos check actually will match any punctuation character no matter the position because the real issue is that we are losing any reference to the replacement positions when the replacement is nothing.

Edited August 28 by poljpocket