Skip to content

fix: Adjust code to avoid PHP8 messages #41597

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DeepDiver1975 merged 3 commits into from
Jun 8, 2026
Merged

fix: Adjust code to avoid PHP8 messages #41597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
592e331
fix: avoid trying to access array offset on false
phil-davis May 20, 2026
107d87f
fix: handle passing null to normalizeUrl
phil-davis May 20, 2026
de8166c
chore: changelog for minor code fixes
phil-davis Jun 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/federation/lib/DbHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ protected function hash($url) {
* @return string
*/
protected function normalizeUrl($url) {
if ($url === null) {

@jvillafanez jvillafanez Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@phil-davis phil-davis Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the phpdoc says that a string should be passed.

Either:

  1. document that null is OK to pass, and is equivalent to the empty string,
    Or
  2. we actually add string as the parameter type in the code, have PHP_ crash when null is passed, and fix all the callers that might pass null.

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.

@DeepDiver1975 DeepDiver1975 Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$url = '';
}
$normalized = $url;

if (\strpos($url, 'https://') === 0) {
Expand Down
7 changes: 7 additions & 0 deletions changelog/unreleased/41597
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Fix: Adjust code to avoid PHP8 messages

Avoid trying to access array offset on false in the encryption storage wrapper.

Handle passing null to normalizeUrl in the federation DbHandler.

https://github.com/owncloud/core/pull/41597
2 changes: 1 addition & 1 deletion lib/private/Files/Storage/Wrapper/Encryption.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ public function fopen($path, $mode) {
if (!empty($encryptionModuleId)) {
$encryptionModule = $this->encryptionManager->getEncryptionModule($encryptionModuleId);
$shouldEncrypt = true;
} elseif (empty($encryptionModuleId) && $info['encrypted'] === true) {
} elseif ($info !== false && $info->isEncrypted()) {
// we come from a old installation. No header and/or no module defined
// but the file is encrypted. In this case we need to use the
// OC_DEFAULT_MODULE to read the file
Expand Down
Loading