From b946b201931bd3519d7aa7d2869a91c5e3cb11bf Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 1 Nov 2020 13:07:00 -0800 Subject: [PATCH] Avoid pass-by-reference issues in config parsing leading to duplicated responses; ref #2511 --- app/Services/Eggs/EggConfigurationService.php | 63 +++++++++---------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/app/Services/Eggs/EggConfigurationService.php b/app/Services/Eggs/EggConfigurationService.php index e7d01df2e..6dbb469ba 100644 --- a/app/Services/Eggs/EggConfigurationService.php +++ b/app/Services/Eggs/EggConfigurationService.php @@ -104,47 +104,33 @@ class EggConfigurationService // can property map the egg placeholders to values. $structure = $this->configurationStructureService->handle($server, true); - foreach ($configs as $file => &$data) { - $this->iterate($data->find, $structure); - } - $response = []; // Normalize the output of the configuration for the new Wings Daemon to more // easily ingest, as well as make things more flexible down the road. foreach ($configs as $file => $data) { - $append = ['file' => $file, 'replace' => []]; + $append = array_merge((array)$data, ['file' => $file, 'replace' => []]); + + foreach ($this->iterate($data->find, $structure) as $find => $replace) { + if (is_object($replace)) { + foreach ($replace as $match => $replaceWith) { + $append['replace'][] = [ + 'match' => $find, + 'if_value' => $match, + 'replace_with' => $replaceWith, + ]; + } - /** @noinspection PhpParameterByRefIsNotUsedAsReferenceInspection */ - // I like to think I understand PHP pretty well, but if you don't pass $value - // by reference here, you'll end up with a resursive array loop if the config - // file has two replacements that reference the same item in the configuration - // array for the server. - foreach ($data as $key => &$value) { - if ($key !== 'find') { - $append[$key] = $value; continue; } - foreach ($value as $find => $replace) { - if (is_object($replace)) { - foreach ($replace as $match => $replaceWith) { - $append['replace'][] = [ - 'match' => $find, - 'if_value' => $match, - 'replace_with' => $replaceWith, - ]; - } - - continue; - } - - $append['replace'][] = [ - 'match' => $find, - 'replace_with' => $replace, - ]; - } + $append['replace'][] = [ + 'match' => $find, + 'replace_with' => $replace, + ]; } + unset($append['find']); + $response[] = $append; } @@ -248,21 +234,28 @@ class EggConfigurationService * * @param mixed $data * @param array $structure + * @return mixed */ - private function iterate(&$data, array $structure) + private function iterate($data, array $structure) { if (! is_iterable($data) && ! is_object($data)) { - return; + return $data; } - foreach ($data as &$value) { + // Remember, in PHP objects are always passed by reference, so if we do not clone this object + // instance we'll end up making modifications to the object outside the scope of this function + // which leads to some fun behavior in the parser. + $clone = clone $data; + foreach ($clone as $key => &$value) { if (is_iterable($value) || is_object($value)) { - $this->iterate($value, $structure); + $value = $this->iterate($value, $structure); continue; } $value = $this->matchAndReplaceKeys($value, $structure); } + + return $clone; } }