Skip to content

FIX: Check $test is not null before use in strpos() for Notices #318

Open
brodiefarrell wants to merge 1 commit intoprocesswire:devfrom
brodiefarrell:fix/notices-strpos-null-check
Open

FIX: Check $test is not null before use in strpos() for Notices #318
brodiefarrell wants to merge 1 commit intoprocesswire:devfrom
brodiefarrell:fix/notices-strpos-null-check

Conversation

@brodiefarrell
Copy link
Copy Markdown

@brodiefarrell brodiefarrell commented Apr 1, 2025 •
edited
Loading

In Notices.php, there is a chance that $text (derived from $item->text) can be used in strpos whilst being null. This causes a warning in PHP 8.

@brodiefarrell brodiefarrell marked this pull request as ready for review April 1, 2025 04:18
@poljpocket
Copy link
Copy Markdown
Contributor

poljpocket commented Jul 1, 2025

Isn't the question this: Does a Notice with null as text even make sense?

Most of the code in Notices.php assumes text is a string, an array or an Object. But never null.

By this logic, there are also places in Notices.php which are problematic:

processwire/wire/core/Notices.php

Lines 415 to 427 in 44fcf13

public function __toString() {
$text = $this->text;
if(is_object($text)) {
$value = method_exists($text, '__toString') ? (string) $text : '';
$class = $text->className();
$text = "object:$class";
if($value !== '' && $value !== $class) $text .= "($value)";
} else if(is_array($text)) {
$text = 'array(' . count($text) . ')';
}
return $text;
}
}

__toString() has to return a string value by definition.

processwire/wire/core/Notices.php

Lines 924 to 935 in 44fcf13

protected function noticeToStr(Notice $item) {
$type = str_replace('Notice', '', $item->className());
$a = array(
'type' => $type,
'flags' => $item->flags,
'timestamp' => $item->timestamp,
'class' => $item->class,
'icon' => $item->icon,
'text' => $item->text,
);
return implode(';', $a);
}

This possibly doesn't work as intended when text is null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@brodiefarrell @poljpocket