fix(UserConfig): cast getTypedValue() result to string in getValueBool()

[thereisnotime]

• Resolves: [Bug]: NC 33 / PHP 8.4: TypeError in UserConfig::getValueBool() — strtolower() receives non-string from getTypedValue() #59629

Summary

PHP 8.4 made passing non-strings to strtolower() a fatal TypeError. UserConfig::getValueBool() calls strtolower() directly on the return value of getTypedValue(), which can return a non-string under certain conditions. The (string) cast prevents the TypeError regardless of what getTypedValue() returns.

Verified in production on NC 33.0.2 / PHP 8.4 — LDAP users were getting 500 errors on every authenticated request.

TODO

• Investigate the root cause of why getTypedValue() returns a non-string (see discussion in [Bug]: NC 33 / PHP 8.4: TypeError in UserConfig::getValueBool() — strtolower() receives non-string from getTypedValue() #59629)

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation has been updated or is not required
• Backports requested where applicable — backport to stable33 needed
• Labels added where applicable
• Milestone added for target branch/version

AI (if applicable)

• The content of this PR was partly or fully generated using AI

[artonge]

Isn't it the same as #59630 ?

[CarlSchwan]

Isn't it the same as #59630 ?

yes but the diff is clean

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[thereisnotime]

@artonge @CarlSchwan @nickvergessen @susnux this is still broken in 33.0.3 which shipped April 30. Every authenticated request returns 500. We have this patched manually in production right now as a workaround — without it the entire instance is down.

This is not an edge case. Multiple users confirmed this in the linked issues weeks ago. The PR has been sitting open since April 15 with no merge, no backport, no release. That's three weeks of a known production-breaking regression going unaddressed.

If you're not merging this, explain why. If there's a blocker, say so. Leaving it open with zero progress while people's production environments are on fire is not acceptable.

[susnux]

If you're not merging this, explain why. If there's a blocker, say so. Leaving it open with zero progress while people's production environments are on fire is not acceptable.

It seems from the issue this is solved for people if they restart their containers so this more seems like a php caching issue which would make sense based on the wrong error code shown.
Moreover we are still waiting for someone providing some debug information - see my comment in the issue.
As you say you can still reproduce the issue, you maybe can provide the requested information

[nickvergessen]

@thereisnotime Got time to answer the question so this can be included in the May updates if we have the explanation?

[thereisnotime]

Still reproducible on 33.0.3 without the manual patch. A few notes on susnux's caching theory:

• Restarting containers does clear the symptom, but it comes back immediately after the first authenticated request — consistent with the getValueBool() type error hitting at runtime, not a stale opcode cache.
• The patch (casting getTypedValue() result to string before the bool coercion) eliminates the 500s entirely without a restart.

Happy to pull together reproduction steps or env details if that helps move this forward for the May update.

[nickvergessen]

Happy to pull together reproduction steps or env details

Maybe you can share a backup of the affected rows. I only see code that writes '1' in old versions for user_ldap isDeleted. And the new setValueBool also is called with true which also writes '1' only.

If you could run the following 2 database queries and share the result with us (e.g. via email to <my github handle>@nextcloud.com) that would be very helpful to understand the root cause of the issue:

SELECT * FROM oc_preferences WHERE appid = 'user_ldap' AND configkey = 'isDeleted' AND configvalue != '1';

-- Replace $USERID with the id of an affected user
SELECT * FROM oc_preferences WHERE appid = 'user_ldap' AND userid = '$USERID';

[susnux]

Just a heads up if you not subscribed to the issue:
This is a confirmed PHP opcache bug, its not specific to this line in the code but can happen for every place where a string is passed as reference between functions.
Meaning the safest solution is to either disable opcache or use PHP 8.3 for the time until the PHP bug is fixed (PR available in php/php-src#21973 )

[thereisnotime]

Ran both queries on a production instance running 33.0.3.

Query 1 (configvalue != '1'): zero rows. Every user_ldap isDeleted entry is stored as the string '1'. No unexpected values.

Query 2 (full row for an affected user): isDeleted = '1', alongside the usual displayName, foundDeleted, homePath keys. Nothing unusual at the data layer.

This confirms susnux's opcache diagnosis — the data is fine, the corruption happens at the PHP type level as the value passes through getTypedValue(). The (string) cast re-asserts the type before the comparison and survives the opcache corruption.

We are already running the patched UserConfig.php (with the (string) cast at line 688) in production and have had zero 500s since. Before the patch, every authenticated request triggered the error. After applying it, the instance has been stable.

Happy to provide further info if needed.

[nickvergessen]

In that case I would add an explicit comment in the line above referencing the PHP issue or PR and (if known) affected PHP versions, so it is not removed (by psalm or other automation), until we require a higher PHP version