fix: Adjust code to avoid PHP8 messages - #41597
Conversation
This fixes log file messages like
{"reqId":"rRziQ2UaFono1r96TEdK","level":3,"time":"2026-05-19T12:44:35+00:00","remoteAddr":"::1",
"user":"Brian","app":"PHP","method":"PUT",
"url":"\/server\/remote.php\/webdav\/textfile0.txt-chunking-42-3-2",
"message":"Trying to access array offset on false at
\/var\/www\/html\/server\/lib\/private\/Files\/Storage\/Wrapper\/Encryption.php#475"}
This will fix log file messages like
{"reqId":"oAKAHIupnZUz4ujYoFqY","level":3,"time":"2026-05-19T12:46:31+00:00","remoteAddr":"::1",
"user":"--","app":"PHP","method":"POST",
"url":"\/server\/ocs\/v1.php\/apps\/federation\/api\/v1\/request-shared-secret",
"message":"strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated
at \/var\/www\/html\/server\/apps\/federation\/lib\/DbHandler.php#298"}
|
While getting acceptance tests running, these were 2 things that I noticed that polluted the owncloud log file. |
| * @return string | ||
| */ | ||
| protected function normalizeUrl($url) { | ||
| if ($url === null) { |
There was a problem hiding this comment.
My worry is that the null value comes from above and it could affect to other places. All the public functions in this file assume the url is a string, so it might cause problems if it isn't, not just here but in all this file.
If it's too much to handle at the moment, (we'll probably need to check where all the public methods are being called), at least we should adjust the phpdoc allow null values. Otherwise it feels weird that we supposed to have strings but we're checking for null.
There was a problem hiding this comment.
Yes, the phpdoc says that a string should be passed.
Either:
- document that
nullis OK to pass, and is equivalent to the empty string,
Or - we actually add
stringas the parameter type in the code, have PHP_ crash whennullis passed, and fix all the callers that might passnull.
Maybe it is ok to do (1). Because it looks “reasonable” that the function can easily handle null and "normalize" it to the empty string.
Up to you guys. I have just put this here as the simple way to avoid the messages in the log file.
There was a problem hiding this comment.
pre php8 strpos was converting a null value to an empty string - so with this change we are explicitly doing what was being done implicit before. no issue from my pov
DeepDiver1975
left a comment
There was a problem hiding this comment.
Code Review — fix: Adjust code to avoid PHP8 messages
Overview: Two independent PHP 8 compatibility fixes in a single PR:
Encryption.php:475—$infocould befalsewhengetCache()->get()fails; accessing$info['encrypted']onfalseproduces"Trying to access array offset on false".DbHandler.php:normalizeUrl()— caller can passnull;strpos(null, ...)is deprecated in PHP 8.
Fix 1 — Encryption.php
- } elseif (empty($encryptionModuleId) && $info['encrypted'] === true) {
+ } elseif ($info !== false && $info->isEncrypted()) {This changes two things simultaneously:
- Adds the
$info !== falseguard ✅ - Switches from array access
$info['encrypted']to method call$info->isEncrypted()✅
The method call is the correct approach — $info is a CacheEntry object, not a plain array. The old code using $info['encrypted'] was using array access on an object (which works via ArrayAccess but is inconsistent). Using isEncrypted() is cleaner and avoids the false issue entirely.
Semantic equivalence: $info['encrypted'] === true and $info->isEncrypted() should be equivalent for CacheEntry — worth confirming isEncrypted() exists and returns a boolean on this object. Based on the ownCloud codebase pattern for CacheEntry, this is expected to be correct. ✅
Fix 2 — DbHandler.php
if ($url === null) {
$url = '';
}Converts null to empty string before strpos(). This prevents the PHP 8 deprecation warning. Empty string input to normalizeUrl() will return an empty string, which is the correct behavior for a null URL. ✅
Alternative: A one-liner $url = $url ?? ''; would be idiomatic PHP, but the explicit if is equally correct.
Summary
| Aspect | Assessment |
|---|---|
| PHP 8 array-offset-on-false | ✅ Fixed via $info !== false guard + method call |
| PHP 8 null-to-strpos | ✅ Fixed via null coercion to empty string |
| No tests added | ℹ️ These are defensive guards for edge cases; acceptable without dedicated tests |
| Changelog | ✅ Present |
Verdict: Ready to merge.
fix: avoid trying to access array offset on false
This fixes log file messages like:
fix: handle passing null to normalizeUrl
This will fix log file messages like