Allow double JSON pointers in configuration files.

[aaronweeden]

Description

This PR fixes a bug specific to PHP <7.3 in which configuration files cannot contain double JSON pointers (pointers to pointers). For example, if a configuration file a.json contains this:

{
    "$ref": "b.json"
}

and b.json contains this:

{
    "$ref": "c.json"
}

and c.json contains this:

{
    "d": "e"
}

then running Configuration::factory('a.json') in PHP <7.3, the resulting object would be:

{
    "$ref": {
        "d": "e"
    }
}

instead of the desired:

{
    "d": "e"
}

The reason this bug is happening is due to the code in Configuration::processKeyTransformers(). A simpler piece of code that reproduces the bug is below.

<?php
function transform(&$value) {
    if ($value === 'b.json') {
        $value = (object)['$ref' => 'c.json'];
    } elseif ($value === 'c.json') {
        $value = (object)['d' => 'e'];
    }
}
function processKeyTransformers($obj) {
    foreach ($obj as $key => &$value) {
        if ($key === '$ref') {
            transform($value);
            $obj = $value;
            if (is_object($value)) {
                $value = processKeyTransformers($value);
            }
        }
    }
    return $obj;
}
$result = processKeyTransformers((object)['$ref' => 'b.json']);
echo 'Final result is: ';
var_dump($result);

Pasting this code into https://3v4l.org/, choosing + include eol (slow), and clicking eval();, it shows the different output for PHP <7.3 and PHP ≥7.3:

Output for 7.3.0 - 7.3.33, 7.4.0 - 7.4.33, 8.0.0 - 8.0.30, 8.1.0 - 8.1.28, 8.2.0 - 8.2.19, 8.3.0 - 8.3.7
Final result is: object(stdClass)#1 (1) {
  ["d"]=>
  string(1) "e"
}

Output for 7.0.0 - 7.0.33, 7.1.0 - 7.1.33, 7.2.0 - 7.2.34
Final result is: object(stdClass)#2 (1) {
  ["$ref"]=>
  object(stdClass)#1 (1) {
    ["d"]=>
    string(1) "e"
  }
}

Running the same code with some var_exports to see what is going on:

<?php
function transform(&$value) {
    if ($value === 'b.json') {
        $value = (object)['$ref' => 'c.json'];
    } elseif ($value === 'c.json') {
        $value = (object)['d' => 'e'];
    }
}
function processKeyTransformers($obj) {
    foreach ($obj as $key => &$value) {
        echo 'Looping with $key = ' . var_export($key, true) . ' and value = ' . var_export($value, true) . "\n";
        if ($key === '$ref') {
            transform($value);
            $obj = $value;
            if (is_object($value)) {
                echo "Recurring\n";
                $value = processKeyTransformers($value);
                echo 'Returned, $obj = ' . var_export($obj, true) . "\n";
            }
        }
    }
    return $obj;
}
$result = processKeyTransformers((object)['$ref' => 'b.json']);
echo 'Final result is: ';
var_dump($result);

it can be seen that in PHP ≥7.3, the foreach loops back over $obj after it has been replaced by $value, whereas in PHP <7.3 it does not:

Output for 7.3.0 - 7.3.33, 7.4.0 - 7.4.33, 8.0.0 - 8.0.30, 8.1.0 - 8.1.28, 8.2.0 - 8.2.19, 8.3.0 - 8.3.7
Looping with $key = '$ref' and value = 'b.json'
Recurring
Looping with $key = '$ref' and value = 'c.json'
Recurring
Looping with $key = 'd' and value = 'e'
Returned, $obj = (object) array(
   'd' => 'e',
)
Looping with $key = 'd' and value = 'e'
Returned, $obj = (object) array(
   '$ref' => 
  (object) array(
     'd' => 'e',
  ),
)
Looping with $key = '$ref' and value = (object) array(
   'd' => 'e',
)
Recurring
Looping with $key = 'd' and value = 'e'
Returned, $obj = (object) array(
   'd' => 'e',
)
Looping with $key = 'd' and value = 'e'
Final result is: object(stdClass)#1 (1) {
  ["d"]=>
  string(1) "e"
}

Output for 7.0.0 - 7.0.33, 7.1.0 - 7.1.33, 7.2.0 - 7.2.34
Looping with $key = '$ref' and value = 'b.json'
Recurring
Looping with $key = '$ref' and value = 'c.json'
Recurring
Looping with $key = 'd' and value = 'e'
Returned, $obj = stdClass::__set_state(array(
   'd' => 'e',
))
Returned, $obj = stdClass::__set_state(array(
   '$ref' => 
  stdClass::__set_state(array(
     'd' => 'e',
  )),
))
Final result is: object(stdClass)#2 (1) {
  ["$ref"]=>
  object(stdClass)#1 (1) {
    ["d"]=>
    string(1) "e"
  }
}

However, logically what we want to happen is that if $obj was replaced, it should immediately return instead of continuing to loop, i.e.:

<?php
function transform(&$value) {
    if ($value === 'b.json') {
        $value = (object)['$ref' => 'c.json'];
    } elseif ($value === 'c.json') {
        $value = (object)['d' => 'e'];
    }
}
function processKeyTransformers($obj) {
    $replaceObjWithValue = false;
    foreach ($obj as $key => &$value) {
        if ($key === '$ref') {
            transform($value);
            $replaceObjWithValue = true;
            if (is_object($value)) {
                $value = processKeyTransformers($value);
            }
        }
        if ($replaceObjWithValue) {
            return $value;
        }
    }
    return $obj;
}
$result = processKeyTransformers((object)['$ref' => 'b.json']);
echo 'Final result is: ';
var_dump($result);

It can be seen that this produces the same desired result no matter the PHP ≥7 version:

Output for 5.4.0 - 5.4.45, 5.5.0 - 5.5.38, 5.6.0 - 5.6.40, 7.0.0 - 7.0.33, 7.1.0 - 7.1.33, 7.2.0 - 7.2.34, 7.3.0 - 7.3.33, 7.4.0 - 7.4.33, 8.0.0 - 8.0.30, 8.1.0 - 8.1.28, 8.2.0 - 8.2.19, 8.3.0 - 8.3.7
Final result is: object(stdClass)#3 (1) {
  ["d"]=>
  string(1) "e"
}

This PR makes such a fix to the processKeyTransformers() method.

Motivation and Context

This issue was noticed when adding the new OnDemand realm to ACCESS XDMoD. The PRs that add the OnDemand realm to ACCESS XDMoD (ubccr/xdmod-ondemand#50 and https://github.com/ubccr/xdmod-xsede/pull/473) use a configuration that uses the new JsonReferenceWithFallbackTransformer (introduced in #1852) which refers to a file (configuration/etl/etl_data.d/ood/request-path-filter.d/Texas-A&M-U-ACES-OnDemand.json) that has a $ref-with-overwrite to another file (configuration/etl/etl_data.d/ood/request-path-filter.json).

Tests performed

This PR extends an existing unit test to have it check for a double JSON pointer in a configuration file. I also tested ingesting logs on my port on xdmod-dev with the changes from ubccr/xdmod-ondemand#50, https://github.com/ubccr/xdmod-xsede/pull/473, and the PRs on which they depend.

Checklist:

• The pull request description is suitable for a Changelog entry
• The milestone is set correctly on the pull request
• The appropriate labels have been added to the pull request

[jpwhite4]

Please can you double check this works with the php 8 version in rocky (perhaps merge ryans php8 changes into a (temporary) new pull request? The plan is for php8 as the supported version for 11

[aaronweeden]

Please can you double check this works with the php 8 version in rocky (perhaps merge ryans php8 changes into a (temporary) new pull request? The plan is for php8 as the supported version for 11

@jpwhite4 I have done so here: #1857