Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

alternatives for "clone $GLOBALS" ? #78

Open
mario-seidel opened this issue Mar 18, 2020 · 5 comments
Open

alternatives for "clone $GLOBALS" ? #78

mario-seidel opened this issue Mar 18, 2020 · 5 comments
Labels
Milestone

Comments

@mario-seidel
Copy link

Can we think about this again?

e00f12a#diff-85035a0cade545b687554eb75b5a191cR112

@TehTux
Copy link
Member

TehTux commented Mar 24, 2020

Of course, can you give me input? I'm not familiar with the issue.

What is the issue? How can I reproduce it?

@rengaw83
Copy link

rengaw83 commented Mar 26, 2020

Hi,
the problem is, that $GLOBALS is an array and can't be cloned.

This code results in an fatal error:

$copy = clone $GLOBALS;

Output in ALL PHP versions:

Fatal error: Uncaught Error: __clone method called on non-object

To clone an array you just need to assign it to a new variable.

But $GLOBALS has a peculiarity, see https://www.php.net/manual/de/reserved.variables.globals.php#112395
The keys of $GLOBALS are always an reference!

To create a clone of the $GLOBALS array you can perform an array copy.

Here are a demonstration of this issue (or run it on https://3v4l.org/VkDgZ):

// Testing Globals
$GLOBALS['A'] = 'B';

$arrayCopyGlobalsVar = $GLOBALS;
$arrayCopyGlobalsVar['A'] = 'C';

var_dump([
    '$GLOBALS' => $GLOBALS['A'], 
    '$arrayCopyGlobalsVar' => $arrayCopyGlobalsVar['A'], 
]);

$GLOBALS['A'] = 'B';

$nonReferencedGlobalsVar = new ArrayObject($GLOBALS);
$nonReferencedGlobalsVar = $nonReferencedGlobalsVar->getArrayCopy();
$nonReferencedGlobalsVar['A'] = 'D';

var_dump([
    '$GLOBALS' => $GLOBALS['A'], 
    '$nonReferencedGlobalsVar' => $nonReferencedGlobalsVar['A']
]);

Output for PHP 5.4.0 - 7.4.4:

array(2) {
  ["$GLOBALS"]=>
  string(1) "C"
  ["$arrayCopyGlobalsVar"]=>
  string(1) "C"
}
array(2) {
  ["$GLOBALS"]=>
  string(1) "B"
  ["$nonReferencedGlobalsVar"]=>
  string(1) "D"
}

i can't understand how e00f12a could be done and why this should fix #58.

I think you can reproduce this by checking an extension value like mentioned in #58.

But i think you can replace this clone by doing this:

-                    $value = clone $GLOBALS;
+                    $value = (new ArrayObject($GLOBALS))->getArrayCopy();

@mario-seidel
Copy link
Author

mario-seidel commented Mar 26, 2020

The main problem is, that a method called "getValueForKeyPath" is modifying the global state:

if ($keyPath[0] == 'TYPO3_CONF_VARS' && $keyPath[1] == 'EXT' && $keyPath[2] == 'extConf' && $keyPath[3]) {
    $value = clone $GLOBALS;
    $serializedValue = $value[$keyPath[0]][$keyPath[1]][$keyPath[2]][$keyPath[3]];
    $value[$keyPath[0]][$keyPath[1]][$keyPath[2]][$keyPath[3]] = unserialize($serializedValue);
}

With the if check the structure of the return array is clear. So we don't have to overwrite the $GLOBALS here. Instead we can return an array with the serialized data like this:

//untested code
if ($keyPath[0] == 'TYPO3_CONF_VARS' && $keyPath[1] == 'EXT' && $keyPath[2] == 'extConf' && $keyPath[3]) {
    $serializedValue = $GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf'][$keyPath[3]];
    $value = [
        'TYPO3_CONF_VARS' => [
            'EXT' => [
                'extConf' => [
                    $keyPath[3] => unserialize($serializedValue)
                ]
            ]
        ]
    ];
}

In fact, this is much more readable and do what the method name pretends.

@TehTux TehTux added the bug label Apr 21, 2020
@TehTux
Copy link
Member

TehTux commented Apr 21, 2020

Hey guys,

thanks for your detailed feedback. Do you need a backport for 2.x?

@mario-seidel
Copy link
Author

There are still some needs for it so it would be really nice to have a backport.

@TehTux TehTux added this to the 3.0.1 milestone Apr 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants