Skip to content

PHP8: SimplePie wrong use of isset#3404

Merged
Alkarex merged 4 commits intoFreshRSS:masterfrom
Alkarex:simplepie_fix_isset
Jan 31, 2021
Merged

PHP8: SimplePie wrong use of isset#3404
Alkarex merged 4 commits intoFreshRSS:masterfrom
Alkarex:simplepie_fix_isset

Conversation

@Alkarex
Copy link
Copy Markdown
Member

@Alkarex Alkarex commented Jan 29, 2021

#fix #3401 (crash with PHP 8+)

ceil() crashes in PHP8+ in case of invalid input such as empty string.
intval() fixes the problem with almost identical behaviour than ceil() in PHP7- (except for floating point values)

#fix #3401 (crash with PHP 8+)

Example with feed http://podcast.hr2.de/derTag/podcast.xml

<enclosure url="https://mp3podcasthr-a.akamaihd.net:443/mp3/podcast/derTag/derTag_20210129_87093232.mp3"
length="" type="audio/mpeg"/>

isset("") passes and then ceil("") crashes due to wrong type in PHP8+:

Uncaught TypeError: ceil(): Argument #1 ($num) must be of type
int|float, string given in ./SimplePie/SimplePie/Item.php:2871

#fix FreshRSS#3401
Use `!empty` instead to properly handle falsy values such as empty
string, 0, false, null.
While 0 would be grammatically fine, it does not make sense for a media
length or height, or width or framerate or bitrate
@Alkarex Alkarex added this to the 1.18.0 milestone Jan 29, 2021
@Alkarex Alkarex changed the title SimplePie wrong use of isset PHP8: SimplePie wrong use of isset Jan 29, 2021
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Jan 29, 2021

Patch sent upstream simplepie/simplepie#670

if (isset($enclosure[0]['attribs']['']['length']))
if (!empty($enclosure[0]['attribs']['']['length']))
{
$length = ceil($enclosure[0]['attribs']['']['length']);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here was the actual crash with PHP8+

@Alkarex Alkarex merged commit 45ee7a3 into FreshRSS:master Jan 31, 2021
@Alkarex Alkarex deleted the simplepie_fix_isset branch January 31, 2021 12:04
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Jan 31, 2021

Merged upstream

Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Feb 20, 2021
Alkarex added a commit that referenced this pull request Feb 20, 2021
* Manual update to SimplePie 1.5.6

Follow-up of #3206 (1.5.5)
Differences
simplepie/simplepie@692e8bc...155cfcf
Related to #3416 ,
#3404

* Typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uncaught TypeError: ceil(): Argument #1 ($num) must be of type int|float, string given

1 participant