[wp-trac] [WordPress Trac] #53278: [PHP 8] RSS Widget has infinite loop for instances with an undefined URL.

[WordPress Trac]

#53278: [PHP 8] RSS Widget has infinite loop for instances with an undefined URL.
-----------------------------------+---------------------
 Reporter:  peterwilsoncc          |       Owner:  (none)
     Type:  defect (bug)           |      Status:  new
 Priority:  normal                 |   Milestone:  5.8
Component:  Widgets                |     Version:
 Severity:  normal                 |  Resolution:
 Keywords:  php8 needs-unit-tests  |     Focuses:
-----------------------------------+---------------------
Changes (by hellofromTonya):

 * keywords:  php8 => php8 needs-unit-tests


Comment:

 In reviewing with @jrf, yes `substr()` returns an empty string in PHP 8.0
 whereas with < PHP 8.0, it returned `false`. The patch is checking for a
 falsey state instead of checking specifically for `''` or `false`. In this
 instance checking against both values adds more code to the `while` which
 increases the cognitive complexity. Instead `empty()` is appropriate. It
 allows the next person who comes along to understand what the check is for
 and why.

 As the tests weren't failing on this issue when they were enabled for PHP
 8.0, we can only conclude that this piece of code does not have test
 coverage. We strongly recommend that tests be added to prevent this issue
 from being reintroduced.

 As for SimplePie, based on the code snippet, we are 100% sure it will be
 affected by this. Using `empty()` however is not a viable solution because
 the port may be string `'0'`, which would not work with `empty()`. For
 SimplePie, checks against empty string and boolean `false` are needed. The
 assignment also needs to be moved out of the conditional expression for
 this to work. Why? Older versions of PHP (which are supported by
 SimplePie) do not allow an expression to be passed into `empty()`.

 @dd32 Do you want to create the issue and PR upstream for SimplePie? Or
 would you prefer us to do it?

 @peterwilsoncc Do you have time to write the tests for this patch?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/53278#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform