From 0754de8d1bfd0101dd4d7eb8bd3591b797166330 Mon Sep 17 00:00:00 2001
From: Sami Mazouz <sychocouldy@gmail.com>
Date: Sun, 6 Dec 2020 20:58:45 +0100
Subject: [PATCH] Pass callback wrapper parameters by reference (#2485)

Because invokable class objects are not directly called and instead it's the callback wrapper that calls these objects, it's currently not possible to receive arguments by reference on an invokable class.

To fix this we pass the arguments by reference by default when calling the object in the callback wrapper.
---
 .../core/src/Foundation/ContainerUtil.php     |   4 +-
 .../unit/Foundation/ContainerUtilTest.php     | 140 ++++++++++++++++++
 2 files changed, 142 insertions(+), 2 deletions(-)
 create mode 100644 framework/core/tests/unit/Foundation/ContainerUtilTest.php

diff --git a/framework/core/src/Foundation/ContainerUtil.php b/framework/core/src/Foundation/ContainerUtil.php
index bc566802d..1c5b95972 100644
--- a/framework/core/src/Foundation/ContainerUtil.php
+++ b/framework/core/src/Foundation/ContainerUtil.php
@@ -24,10 +24,10 @@ class ContainerUtil
     public static function wrapCallback($callback, Container $container)
     {
         if (is_string($callback)) {
-            $callback = function () use ($container, $callback) {
+            $callback = function (&...$args) use ($container, $callback) {
                 $callback = $container->make($callback);
 
-                return call_user_func_array($callback, func_get_args());
+                return $callback(...$args);
             };
         }
 
diff --git a/framework/core/tests/unit/Foundation/ContainerUtilTest.php b/framework/core/tests/unit/Foundation/ContainerUtilTest.php
new file mode 100644
index 000000000..f7b8a2992
--- /dev/null
+++ b/framework/core/tests/unit/Foundation/ContainerUtilTest.php
@@ -0,0 +1,140 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Tests\unit\Foundation;
+
+use Flarum\Foundation\ContainerUtil;
+use Flarum\Tests\unit\TestCase;
+use Illuminate\Container\Container;
+
+class ContainerUtilTest extends TestCase
+{
+    private $container;
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->container = new Container();
+    }
+
+    /** @test */
+    public function it_works_with_closures()
+    {
+        $callback = ContainerUtil::wrapCallback(function ($array) {
+            $array['key'] = 'newValue';
+
+            return 'return';
+        }, $this->container);
+
+        $array = ['key' => 'value'];
+        $return = $callback($array);
+
+        $this->assertEquals('value', $array['key']);
+        $this->assertEquals('return', $return);
+    }
+
+    /** @test */
+    public function it_works_with_invokable_classes()
+    {
+        $callback = ContainerUtil::wrapCallback(CustomInvokableClass::class, $this->container);
+
+        $array = ['key' => 'value2'];
+        $return = $callback($array);
+
+        $this->assertEquals('value2', $array['key']);
+        $this->assertEquals('return2', $return);
+    }
+
+    /** @test */
+    public function it_works_with_invokable_objects()
+    {
+        $callback = ContainerUtil::wrapCallback(new class {
+            public function __invoke($array)
+            {
+                $array['key'] = 'newValue5';
+
+                return 'return5';
+            }
+        }, $this->container);
+
+        $array = ['key' => 'value5'];
+        $return = $callback($array);
+
+        $this->assertEquals('value5', $array['key']);
+        $this->assertEquals('return5', $return);
+    }
+
+    /** @test */
+    public function it_allows_passing_args_by_reference_on_closures()
+    {
+        $callback = ContainerUtil::wrapCallback(function (&$array) {
+            $array['key'] = 'newValue3';
+
+            return 'return3';
+        }, $this->container);
+
+        $array = ['key' => 'value3'];
+        $return = $callback($array);
+
+        $this->assertEquals('newValue3', $array['key']);
+        $this->assertEquals('return3', $return);
+    }
+
+    /** @test */
+    public function it_allows_passing_args_by_reference_on_invokable_classes()
+    {
+        $callback = ContainerUtil::wrapCallback(SecondCustomInvokableClass::class, $this->container);
+
+        $array = ['key' => 'value4'];
+        $return = $callback($array);
+
+        $this->assertEquals('newValue4', $array['key']);
+        $this->assertEquals('return4', $return);
+    }
+
+    /** @test */
+    public function it_allows_passing_args_by_reference_on_invokable_objects()
+    {
+        $callback = ContainerUtil::wrapCallback(new class {
+            public function __invoke(&$array)
+            {
+                $array['key'] = 'newValue6';
+
+                return 'return6';
+            }
+        }, $this->container);
+
+        $array = ['key' => 'value6'];
+        $return = $callback($array);
+
+        $this->assertEquals('newValue6', $array['key']);
+        $this->assertEquals('return6', $return);
+    }
+}
+
+class CustomInvokableClass
+{
+    public function __invoke($array)
+    {
+        $array['key'] = 'newValue2';
+
+        return 'return2';
+    }
+}
+
+class SecondCustomInvokableClass
+{
+    public function __invoke(&$array)
+    {
+        $array['key'] = 'newValue4';
+
+        return 'return4';
+    }
+}