Skip to content

bugfix pdoResources snippet, $output string to placeholders#373

Open
arjen-t wants to merge 1 commit intomodx-pro:masterfrom
arjen-t:master
Open

bugfix pdoResources snippet, $output string to placeholders#373
arjen-t wants to merge 1 commit intomodx-pro:masterfrom
arjen-t:master

Conversation

@arjen-t
Copy link
Copy Markdown

@arjen-t arjen-t commented Sep 22, 2023

Что оно делает?

Check if $ouput is an array before adding the $log result when property $toSeparatePlaceholders is set.

Зачем это нужно?

Uncaught TypeError in PHP >= 8

Связанные проблема(ы)/PR(ы)

#372

@arjen-t arjen-t changed the title bug fix pdoResources snippet, $output string to placeholders bugfix pdoResources snippet, $output string to placeholders Sep 22, 2023
Comment on lines 57 to +63
} elseif (!empty($toSeparatePlaceholders)) {
$output['log'] = $log;
$modx->setPlaceholders($output, $toSeparatePlaceholders);
if(is_array($log)){
$output['log'] = $log;
$modx->setPlaceholders($output, $toSeparatePlaceholders);
}else {
$modx->setPlaceholders(['log' => $log], $toSeparatePlaceholders);
}
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.

The wrong variable is being checked.

  • Problem: $output can be a string (when called via pdoPage with toSeparatePlaceholders), but the code does $output['log'] = $log, which in PHP 8 results in TypeError: Cannot access offset of type string.
  • In the code: $log is always a string (or an empty string), since it is formed as $log .= ' ' . print_r($pdoFetch->getTime(), 1) . ' '.
  • Consequence: the is_array($log) condition is almost always false, the else branch is executed, and only ['log' => $log] is included in placeholders, while the contents of $output are lost.

You need to check $output, not $log:

} elseif (!empty($toSeparatePlaceholders)) {
    if (is_array($output)) {
        $output['log'] = $log;
        $modx->setPlaceholders($output, $toSeparatePlaceholders);
    } else {
        $modx->setPlaceholders(['output' => $output, 'log' => $log], $toSeparatePlaceholders);
    }
}
  • If $output is an array: add log and set all placeholders from $output.
  • If $output is a string: set output and log as separate placeholders.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug PHP 8 pdoResources snippet with property 'toSeparatePlaceholders' in combination with pdoPage

2 participants