PHP 8.4 implicit nullable parameter deprecation in NC31 3rdparty module icewind

ℹ️ Support
, , , , , ,
1

Hi community,

The icewind module contains code that is not compliant with PHP 8.4 due to the deprecation of implicit nullable parameters.
I have corrected some errors in the PHP code using the explicit nullable type. The NC update overwrites these two files, so it would be preferable if the devs could integrate them.

./nextcloud/apps/files_external/3rdparty/icewind/smb/scr/Exception/RevisionMismatchException.php:9
./nextcloud/apps/files_external/3rdparty/icewind/smb/scr/Native/NativeState.php:343

./nextcloud/3rdparty/icewind/searchdav/scr/Query/Scope.php:51
./nextcloud/3rdparty/icewind/searchdav/scr/XML/QueryDiscoverResponse.php:38

Can someone submit a pull request ? The GitHub procedure is beyond me!

(3rdparty/icewind at d93b7897adea5f63ba096a9a49b7c3f8167788df · nextcloud/3rdparty · GitHub)

2

there is some work in this direction already

if you don’t know how to fix - reporting a qualified bug helps already - if you find more issues with php84 compatibility - please create new issue on Github

1 Like
3

As you can see, those are third-party modules which are not maintained by the app maintainer.

Since there are a virtually infinite number of cases of implicit NULL declarations, I wrote a patch tool:

→ phpscan_fix_nullable.php ←

.. that finds all cases of implicit NULL declarations and can fix them in two ways:
both in the short form:

?type $x = null

and in union syntax:

type|null $x = null

Until the entire code, including /3rdparty/ and /vendor/ code, is adapted, which can sometimes take quite a long time, you can simply patch your entire code with the tool as described in the → README ← .
The result is functioning code and a quiet logger. The price can be a complaining Integrity Test. If that bothers you, you can revert the changes to test and then patch again. To revert the changes (e.g. before an update or if you want to run an integrity test) simply run it wirth --undo

I hope you and many others will find the tool valuable!

Much and good luck,

ernolf

2 Likes
4

Thanks, I added this topic as a supplement to the NC31 installation guide I shared in the How-Tos section.

konki

1 Like
5

The first issue is closed for a month already but I still get this issue.

Am I the only one or what should I do?

Can I safely use the patcher provided by ernolf or do I have to do this every update?

I am just annoyed at those errors, but apparently my instance runs fine.

6

technically I suppose the solution by ernolf is good enough but personally I would not mangle with shipped code - especially integrity checks give you strong proof your code is coming unchanged from the devs. once you brake the check you don’t recognize if it was your change or malicious actor. if it bothers you enough I suggest you work through the code and send PRs to resolve the problem at the root.

1 Like
7

Another, and probably the best, option would be to simply stick with PHP 8.3 for a while longer — at least on your production instance — because, after all, that is still the recommended version, and now we also know at least one reason why that is the case. :wink:

And just in case you (or someone else) feels like pointing out: “But Debian 13 ships with PHP 8.4!” — yes, that’s true. But the same principle applies not only to PHP, but also to the OS and all other involved components: on production systems you really shouldn’t jump to the latest versions just for the sake of being “up to date,” without proper testing. Especially not if issues like this one are going to annoy you. :wink:

3 Likes
8

Hi @Tech6767

If you want to stay with php8.4, I would recommend running the patcher over the apps/ directory, as the core code seems to be problem-free so far (from my observations).
After an update, you can run it again. I don’t have any problems with integrity tests, but I’ve already described how to revert everything. After that, all the changes are truly gone without a trace.
Personally, I prefer my log file to remain clear and not to list in deep red that there have been over 4,000 error messages (level 3!) in the last 7 days, which makes it confusing to find the really important messages because they get lost in its multitude.

It is absolutely safe to give it a try. If your live feels better after patching, stay with the result, if not revert the changes or downgrade to php8.3. You can use the php-updater script for that.

You can also just start with one app. Ideally, one that makes a lot of noise, like news, for example:

./phpscan_fix_nullable.php --path=/var/www/nextcloud/apps/news --apply

Then you can observe how it works.

Of course it has to be run as Nextcloud user or a 
sudo chown -R www-data:www-data /var/www/nextcloud

.. has to be appended but that goes without saying I think.

ernolf

9

Thank you for the responses. I am a bit overwhelmed with nextcloud sometimes, usually every bigger update breaks something. I updated a few months ago to 30.xx and needed to upgrade from php 8.0 to a higher version. When I upgraded nextcloud to (I think) 31.xx the web ui was gone. I reverted wirh snapshots in Proxmox, updated to a higher pho version, had to update nextcloud and I think for some reason update another php version, I didn’t write all down but I took a while to update my instance. I think this is nextclouds biggest problem, you assume you can update via the web ui but there are a ton of things you have to think about and the patch notes aren’t user friendly, there is also no newbie friendly warning before upgrading. I guess everything is listed somewhere in the documentation but often it’s just like for example) „install apache“ but doesn’t tell you how.

There is probably a big knowledge gap between people like me and the average nextcloud admin, but I think it’s a pity. Overall I am very happy with Nextcloud and it changed my life for the better :).

Though I just realized, I am not on the recommended php version and that is why I get different errors than most people.

Honestly I am just going to stick to the errors then, it’s only 14 in a week or two, at some point php 8.4 is going to get used and I am going to save myself trouble updating.

My next instance will be running on Docker, assuming I have less things to screw up over there ;).

10

I also prefer that the logs not be filled in unnecessarily so as not to miss out on important information. Active support for php 8.3 ends on December 31, so it’s likely that nextcloud will recommend php 8.4 for version 32 this autumn and will only fix the code from this next version…

I’m thinking of continuing to patch piecemeal with Rector and a simple configuration file as I explain in a more complete how to (nextcloud 31 almalinux 9.6 lemp server-stack).

As with ernolf’s script, patches are overwritten by every update!

11

Before we jump to big conclusions here, I’d like to clarify that, as far as I know, Nextcloud’s own code is mostly ready for PHP 8.4.

I actually see two issues here:

  1. Deprecation messages should only be on log level 0 or maybe 1 — certainly not on 2, and absolutely not on 3. No idea why Nextcloud always has to be so chatty.

  2. The log messages in your original post specifically affect the External Storage app. And no, I don’t think this is about targeting a specific Nextcloud release, because it seems they at least partly depend on upstream (Icewind) to make the External Storage app fully compatible with PHP 8.4.

Since I’m not a developer, I’ll just drop a link here and let you dig deeper to figure out exactly what is what…