[PHP 8.1] Deprecated function: unserialize(): Passing null to parameter #1

Tested and confirmed that this fixes the deprecation error.

First, if you want to fix the PHP 8.1 problems you should create a [meta] issue in this queue - there are going to be more than just this one problem, and this one issue should not be handling multiple problems.

Second, please comment on #3226569: Automated testing configurations. If there is no PHP 8.1 automated testing (or Drupal 9.4.x, or MySql 8, etc.) then there is no way to test patches for PHP 8.1 issues to prevent errors from being introduced into the code base, and no way to test patches like the above to demonstrate that they fix a particular PHP 8.1 issue. I manually triggered a PHP 8.1 test on your patch, and as you can see there are still problems. Because we don't have a test *before* the patch is applied, we can't verify that the patch solved anything.

Third, your patch is not correct. The configuration system is designed so that configuration variables have schema with declared datatypes. If this particular configuration variable does not contain a string then there's a problem in the variable declaration or use. It is not correct to ignore a type error by cast-ing it away.

Use of casts, empty(), isset(), and the like are all indications of an underlying problem that needs to be solved. Ignoring that by making the use of the variable conditional, or by coercing the type when the variable is used, isn't solving the problem - it is just putting the burden on all the code that uses this variable instead of correcting the variable definition. So specifically, this is a string type configuration variable and should never have a non-string value of NULL if it is defined and initialized and used correctly.

This worked for me.

Attached is the patch version.

Hi, strange - I was not able to apply this patch. Patching ActionCommonTrait.php was rejected.
At the end I realized that it is build for 8.x-1.0-alpha3.
But for the latest dev one 8.x-1.x-dev (updated 23 Jan 2022) it needs to be adjusted because there was update of ActionCommonTrait.php
https://git.drupalcode.org/project/content_access/-/blob/8.x-1.0-alpha3/...
https://git.drupalcode.org/project/content_access/-/blob/8.x-1.x/src/Plu...

Here is my slightly updated version which works with latest dev.

Sorry, there was one more change in that file.
New patch attached.

I can confirm that I'm getting this error on the current dev release when running PHP 8.1. The patch seems good, so hopefully it can be merged in.

Works fine!!!. Thanks a lot. D. 9.3.15 , PHP 8.1.3 , 10.5.15-MariaDB-

Automated test still says "Patch Failed to Apply".

Patch #8 also works on D9.4.1, PHP 8.1.6, content_access 1.x-dev@dev

I believe you when you say it works. But we need to figure out why it does not pass the automated tests and reroll to make it pass (or rewrite the test if the test is buggy). This module is horribly complex, and automated tests are necessary to prevent regressions from happening.

I still don't have a test server running PHP 8.1. Until I get around to setting one up, I cannot work on this myself.

Patch was against a drupal installation and not against the module. Fixed and attached.

#Seems to fix the error message

I'm having a hard time debugging the tests, if anyone wants to have a look. It should be the only thing remaining here.

So, I've spend a lot of time on this one. The failing tests are in combination with ACL. I can't tell if it is "ACL" that needs to be fixed or "content_access".

Thing is that this can drive people mad, in the end it lands on core and it needs some effort to hunt it down.

bserem (and the others who have participated in solving this),
thank you so much for your time and effort!

I really want to have a stable release of this, with tests.

Looking back, I believe this project has suffered galloping featuratis. It simply tries to do too much, for too many different use cases. Status today is that there is a lot of technological debt and too little resources to pay it. The project's code base has become very hard and resource intensive to maintain. It is time to take a step back.

AFAIK, the ACL integration never worked right – not even in the Drupal 7 version.

I've already decided to pull the integration with Rules (see #3306154: Restore Content Access Rules Integrations), and the I think we need to pull integration with ACL as well. See #3306205: Remove ACL integration.

I hope that doing so will simplify things, so that the tests will allow your fix for PHP 8.1 to be committed.

hey @gisle, thanks for your reply. I'm all in favor of removing some fat. That said, I do not need/use ACL in my case, so I can't speak for anyone.

If you need ACL, you obviously need to get this fixed, one way or another. AFAIK, nobody else is working on fixing this.

In that case, you may want to check out Flexi Access

It was initially a fork of the ACL integration in this project, because we couldn't get ACL to work with Content Access. I still use it on a few Drupal 7 production sites.

Note: It has not been upgraded to Drupal 9/10, but I suspect that an upgrade shall be less work than getting the ACL integration of Content Access working.

If you want to have a go at it, just let me know, and I'll add you as co-maintainer to Flexi Access. Or, if you want to work on fixing the ACL integration in Content Access instead, I can add you as co-maintainer here (and close the issue about ripping out the ACL integration)

@bserem, thanks for the help! If you've got time to co-maintain, that would be amazing, though I know it's a tough ask.

Why are the configuration settings being serialized and unserialzed in the first place? That seems to be legacy from D7 when things were stored in untyped system variables. Now that we have configuration settings and configuration schema, there is no need to serialize anything in configuration. And as I said above, if you define your settings and your settings schema properly then casts should never be needed.

I think the patch as it stands is just digging a deeper hole - it's not fixing anything, it's just hiding the problem.

@xeM8VfDh thanks for the offer, really appreciate it! I have a very small experience with content_access, from projects I inherited, and I do not feel I can take it further, not now anyway. If at any point I feel comfy about such a task I will not hesitate to ask, but right now is not the right moment for me.

Uploading patch to run tests.

Thank you.
Patch #14 is working

Patch #24 does not contain lots of code from patch #14.
With #14 things are smooth in my sites, with #24 I get a lot of WSOD pages.

Maybe a re-roll based on #14 is a good idea?

I tried the following:
* Rerolling patch #24
To only use arrays the schema of the content access data will have to change to an array from a string.

web/modules/contrib/content_access/config/schema/content_access.schema.yml

     content_access_node_type:
      type: sequence
      label: 'Content Access node type settings'
      sequence:
        type: string
        label: 'Grants'

* Rerolling patch #14
The issue here is the data is saved as a string, but it will not then transform back to an array.

* I tried using $serializer = \Drupal::service('serializer'); instead of the PHP ones but that was no good.

* Then I just saved content access data again.
It changed from a format like:
a:3:{s:8:"view_own";a:2:{i:0;s:12:"site_manager";i:1;s:13:"administrator";}s:4:"view";a:1:{i:0;s:13:"administrator";}s:8:"per_node";i:1;}
To a format like:

a:2:{s:8:"view_own";a:5:{i:0;s:9:"anonymous";i:1;s:13:"authenticated";i:2;s:12:"site_manager";i:3;s:13:"administrator";}s:4:"view";a:5:{i:0;s:9:"anonymous";i:1;s:13:"authenticated";i:2;s:12:"site_manager";i:3;s:13:"administrator";}}

@bserem Hi, I have made investigation further and found an issue why the tests are failing. In fact it's not even related to unserialize issue, but the problem is in ACL module itself. See related issue and patch I added there. After applying the patch from related issue #14 patch no longer fails testing for me.

Hi @jaimekristene
You mentioned there are some potential issues while using patch #14. Could you please specify the cases? I've been doing some testing with that and was not able to identify anything going wrong.

#30 @oleg-chemerys

You're right, The patch in #14 works for me.

TODO:
* Remove the ACL requirement so the tests pass - https://www.drupal.org/project/content_access/issues/3306205
* Create a separate ticket to ask to swap content_access schema to store as an array instead of a string for the content_access_node_type.

This has to be postponed until at least one of these is fixed:

1. #3306205: Remove ACL integration
2. #3324285: [PHP 8.1] Deprecated function: mb_strlen(): Passing null to parameter #1

The first one is for this module and will be committed as soon as a usable patch or merge request exists. The second one depends on the maintainers of ACL commits the existing RTBC patch.

Thank you. Patch #14 worked for me.

Thank you. Patch #14 worked for me.

Thank you.
Patch #14 is working

Thanks, Patch #14 worked for me.

I tested patch #14 on Drupal 9.5 and PHP 8.1 and it works.

Nothing seems to happen at ACL anymore. Should this really be postponed on that? PHP8.1 is the recommended PHP version and this is filling up logs.

I agree with #40

What do you suggest? It is Postponed until ACL is fixed. Is there anything to do but wait until that happens.

@gisle: did you notice my patch about ACL (see issue 3306205)? I got no comment about it.

Yes. I see that #3306205: Remove ACL integration is suggested as an "untested patch", but with no community reviews. It is not on my schedule to review it.

This module is currently working OK for my use cases. I understand that there are some use cases were it is not working, I can't allocate resources to test those out without someone paying me to do so.

@gisle: As I understand #31 the (string) casting in #14 fixes the message and other things can be solved in follow-ups?

That was my point in #40. Perhaps I should have been more clear or I'm missing something here?

Patch #14 on Drupal 9.5.4 and PHP 8.12 worked for me. Thanks to you!!!

This might be related to this core issue: #3300404: Handle nullable serialized field columns

So, patch from #14 works as expected and does fix the issue, but it has an extra safe code that is not needed.

Approach from #24 is a valid try to improve the module by storing config not as serialized string, but like a normal array, but this approach requires a) update to config schema b) hook_update to resave all settings => clearly not the scope of this issue.

Also, postponing this on ACL fixes can be worked around. See the attached patch

Okay, enabling deprecations is not part of the scope either, let's try this minimalistic patch

Huh ..

ACL has been updated, hence the new simple patch

thanks @Taran2L. I can test this later. In any case, should we hold off until the new ACL release it published, and use a new version of your latest patch that doesn't depend on the dev branch of ACL? Or do you think this is ready to go as is?

@gisle, thought?

hey @Taran2L and @hswong3i, I am unable to reproduce this error.

On my dev instance, I have the following in settings.local.php:

# https://www.drupal.org/forum/support/post-installation/2018-07-18/enable-drupal-8-backend-errorlogdebugging-mode
$config['system.logging']['error_level'] = 'verbose';
error_reporting(E_ALL);
ini_set('display_errors', TRUE);
ini_set('display_startup_errors', TRUE);

I am running PHP 8.1, and I have followed the steps in the OP: Navigate to Manage » Structure » Content types. Under operations, Select "Access control" for any content type.

I am not seeing this warning in any of my php/nginx log files. Any advice?

The fix in patch in comment #54 has been pushed to the latest development snapshot.

Hopefully, the fix (#3324285: [PHP 8.1] Deprecated function: mb_strlen(): Passing null to parameter #1) in ACL get committed to a tagged release so that we can go back to using the stable version of ACL again.

thanks @gisle and @Taran2L!

I am working on the ACL issue(s). Savlis popped in to do some work, but seems to have gone absent again. I'm hopefully he will return to clean up a few more loose ends and publish a new release. 🤞