s9y / Serendipity

A PHP blog software
https://s9y.org
BSD 3-Clause "New" or "Revised" License
206 stars 86 forks source link

Net_DNS2 from 2.4.0 is not compatible with PHP8 #805

Closed manitu-opensource closed 7 months ago

manitu-opensource commented 1 year ago

The Net_DNS2 version bundled with 2.4.0 is not (fully) compatible with PHP8.

Especially using the each function (line 923, bundled-libs/Net/DNS2.php) is no longer supported in PHP8.

We suggest to use a newer version from upstream (https://github.com/mikepultz/netdns2/blob/master/Net/DNS2.php)

onli commented 1 year ago

Hey, thanks for the report. And I'm sorry you ran into this, with the non-working serendipity_event_spamblock_rbl plugin.

Using the newer version looks like a good solution, we just have to check it again :)

onli commented 1 year ago

Hi @manitu-opensource

Looking at this dependency, I see no usage of it in the core or in any of the published additional plugins. Do you use or did you see a specific usage of Net_DNS2 with serendipity?

I'm asking so I can test this upgrade. And also, since ideally I'd like to let composer manage this dependency, but then the path would change -> if it's used the old require path should continue to work, but that's a bit hairy because of the other dependencies under bundled_libs/Net (specifically DNSBL). But if there is no real usage of this I'd be okay with making this a breaking change.

Edit: Ah, I get this now. It's specifically bundled_libs/Net/DNSBL that uses Net_DNS2, and DNSBL is used with serendipity_event_spamblock_rbl and serendipity_event_spamblock_surbl. That I can test and patch. I'll report back here, but please let me know of additional usage of this dependency if known :)

onli commented 1 year ago

In my test, https://github.com/s9y/Serendipity/commit/e16533863e198a4ec3bfc45e354de8a135e02065 solved this issue. Upgrade of the dependency as proposed, plus moving it to composer, with a symlink to keep DNSBL working.

hannob commented 1 year ago

Can we close this bug?

onli commented 7 months ago

I'd say so. Please re-open if something has gone wrong with this fix.