phug-php / phug

Phug - The Pug Template Engine for PHP
https://phug.selfbuild.fr
MIT License
62 stars 3 forks source link

array_replace_recursive(): recursion detected when assigning $_SESSION as shared_variables #63

Closed char101 closed 4 years ago

char101 commented 4 years ago

Hello,

I encountered an issue with the following code:

$options = [
    'shared_variables' => ['session' => $_SESSION]
];
$phug = new \Phug\Renderer($options);

The value of $_SESSION contains a reference value

$_SESSION['_flash_'] = &self::$flash;

I expected to get: $session available in the template as shared variables.

But I actually get:

phug/phug/src/Phug/Util/Util/Partial/OptionTrait.php [64]:

array_replace_recursive(): recursion detected

Why do shared_variables have to be processed using array_replace_recursive in the first place?

Thanks!

kylekatarnls commented 4 years ago

Hello,

It's nothing specific to shared_variables, options requires a deep merge. And it's relevant for shared_variables that may also be filled by third-party libraries you may install:

$options = [
    'shared_variables' => ['session' => $_SESSION],
    'modules' => [SomeModule::class], // SomeModule has shared_variables
];
$phug = new \Phug\Renderer($options);
// This will give you:
$options = [
    'shared_variables' => [
        'session' => $_SESSION,
        'someModuleVar' => 'foo',
    ],
];

And the reference is probably not the problem, you actually have in $flash a reference to the session that may create the infinite recursion.

You can do:

$phug = new \Phug\Renderer();
$phug->setOptions([
    'shared_variables' => ['session' => $_SESSION],
]);

This will replace all the shared_variables (in your case: none) with your new array without using array_replace_recursive. I did not verify it but the method ->share('shared_variables', ['session' => $_SESSION]) should be fine too.

char101 commented 4 years ago

I actually uses a \Phug\Renderer instance rather than the static method.

Also the reference array is empty, but it need to be a static variable rather than a local variable. Only then will it emits an array_replace_recursive error.

char101 commented 4 years ago

I have also tried to using the setOptions method rather than passing the shared_variables in the constructor but I still get the array_replace_recursive error.

kylekatarnls commented 4 years ago

Hi I added your case in unit tests and get no error: https://github.com/phug-php/phug/commit/26c815cc93c9f3d0624df60c22d2362cc21bf47f#diff-fa8b6bf67b38687976da2539f0c0fe48R322-R334

Can you please add data content with your code to provide a reproductible showcase.

char101 commented 4 years ago

Hi, here is the test case

pug.php

$data = [];

$session = [
    'data' => &$data
];

$pug = new Pug([
    'shared_variables' => [
        'session' => $session
    ]
]);

echo $pug->render('');

php pug.php

PHP Warning:  array_replace_recursive(): recursion detected in /tmp/vendor/phug/phug/src/Phug/Util/Util/Partial/OptionTrait.php on line 64
PHP Stack trace:
PHP   1. {main}() /tmp/pug.php:0
PHP   2. Pug\Pug->render() /tmp/pug.php:17
PHP   3. Pug\Pug->renderString() /tmp/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:93
PHP   4. call_user_func:{/tmp/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:59}() /tmp/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:59
PHP   5. Pug\Pug->Pug\Engine\{closure:/tmp/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:51-53}() /tmp/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:59
PHP   6. Pug\Pug->renderWithPhp() /tmp/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:52
PHP   7. Pug\Pug->render() /tmp/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:20
PHP   8. Pug\Pug->callAdapter() /tmp/vendor/phug/phug/src/Phug/Renderer/Renderer.php:113
PHP   9. Pug\Pug->handleHtmlEvent() /tmp/vendor/phug/phug/src/Phug/Renderer/Renderer/Partial/AdapterTrait.php:238
PHP  10. Pug\Pug->Phug\Renderer\Partial\{closure:/tmp/vendor/phug/phug/src/Phug/Renderer/Renderer/Partial/AdapterTrait.php:234-236}() /tmp/vendor/phug/phug/src/Phug/Renderer/Renderer/Partial/AdapterTrait.php:176
PHP  11. Pug\Pug->Phug\{closure:/tmp/vendor/phug/phug/src/Phug/Renderer/Renderer.php:116-118}() /tmp/vendor/phug/phug/src/Phug/Renderer/Renderer/Partial/AdapterTrait.php:235
PHP  12. Pug\Pug->compile() /tmp/vendor/phug/phug/src/Phug/Renderer/Renderer.php:117
PHP  13. Phug\Compiler->compile() /tmp/vendor/phug/phug/src/Phug/Renderer/Renderer.php:79
PHP  14. Phug\Formatter->format() /tmp/vendor/phug/phug/src/Phug/Compiler/Compiler.php:733
PHP  15. Phug\Formatter\Format\BasicFormat->setFormatter() /tmp/vendor/phug/phug/src/Phug/Formatter/Formatter.php:559
PHP  16. Phug\Formatter\Format\BasicFormat->setOptionsRecursive() /tmp/vendor/phug/phug/src/Phug/Formatter/Formatter/AbstractFormat.php:161
PHP  17. Phug\Formatter\Format\BasicFormat->setOptionArrays() /tmp/vendor/phug/phug/src/Phug/Util/Util/Partial/OptionTrait.php:98
PHP  18. Phug\Formatter\Format\BasicFormat->withVariableReference() /tmp/vendor/phug/phug/src/Phug/Util/Util/Partial/OptionTrait.php:66
PHP  19. Phug\Formatter\Format\BasicFormat->Phug\Util\Partial\{closure:/tmp/vendor/phug/phug/src/Phug/Util/Util/Partial/OptionTrait.php:62-66}() /tmp/vendor/phug/phug/src/Phug/Util/Util/Partial/OptionTrait.php:152
PHP  20. array_replace_recursive() /tmp/vendor/phug/phug/src/Phug/Util/Util/Partial/OptionTrait.php:64
char101 commented 4 years ago

So I was dumping the parameters to array_replace_recursive and at one time it is trying to merge a very large array which looks like the same variable.

This is the value of $base[$name]

359  $base[$name]
360  array:6 [
361    "time_precision" => 3
362    "line_height" => 30
363    "display" => false
364    "log" => false
365    "dumpevent" => array:2 [
366      0 => Phug\Renderer\Profiler\ProfilerModule {#19
367        -startTime: 1586257373.6645
368        -initialMemoryUsage: 4078920
369        -events: Phug\Renderer\Profiler\EventList {#18
370          -locked: false
371          flag::STD_PROP_LIST: false
372          flag::ARRAY_AS_PROPS: false
373          iteratorClass: "ArrayIterator"
374          storage: []
375        }
376        -nodesRegister: SplObjectStorage {#20
377          storage: []
378        }
379        -eventDump: array:2 [
380          0 => Phug\Renderer\Profiler\ProfilerModule {#19}
381          1 => "getEventDump"
382        ]
383        -container: Pug\Pug {#2
384          #filters: null
385          #optionsAliases: array:6 [
386            "cache" => "cachedir"
387            "prettyprint" => "pretty"
388            "allowMixedIndent" => "allow_mixed_indent"
389            "keepBaseName" => "keep_base_name"
390            "notFound" => "not_found_template"
391            "customKeywords" => "keywords"

While this is the value of $value

2319 $value
2320 array:6 [
2321   "time_precision" => 3
2322   "line_height" => 30
2323   "display" => false
2324   "log" => false
2325   "dumpevent" => array:2 [
2326     0 => Phug\Renderer\Profiler\ProfilerModule {#19
2327       -startTime: 1586257373.6645
2328       -initialMemoryUsage: 4078920
2329       -events: Phug\Renderer\Profiler\EventList {#18
2330         -locked: false
2331         flag::STD_PROP_LIST: false
2332         flag::ARRAY_AS_PROPS: false
2333         iteratorClass: "ArrayIterator"
2334         storage: []
2335       }
2336       -nodesRegister: SplObjectStorage {#20
2337         storage: []
2338       }
2339       -eventDump: array:2 [
2340         0 => Phug\Renderer\Profiler\ProfilerModule {#19}
2341         1 => "getEventDump"
2342       ]
2343       -container: Pug\Pug {#2
2344         #filters: null
2345         #optionsAliases: array:6 [

Looks pretty much the same array and both of these arrays contains references to the shared_variables which contains reference to the same value of $data.

char101 commented 4 years ago

This is the same thing that happens with

$data = [];
$data2 = ['data' => &$data];
array_replace_recursive(['a' => $data2], ['a' => $data2]);
Warning: array_replace_recursive(): recursion detected in /tmp/pug.php on line 22
kylekatarnls commented 4 years ago

It sounds like object are not a problem even with recursion. Only array references are.

It's very unexpected to get this error while there is actually no recursion at all 🤔

kylekatarnls commented 4 years ago

This will be fixed in the next release. You can yet test it with adding "phug/phug": "dev-master" in your compsoer.json.

kylekatarnls commented 4 years ago

1.7.1 released with this fixed.