Skip to content

Commit

Permalink
Fix deprecation on dropdown fields with a null entity restrict
Browse files Browse the repository at this point in the history
  • Loading branch information
cedric-anne committed Mar 21, 2024
1 parent eb40b43 commit f1d2074
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 7 deletions.
4 changes: 3 additions & 1 deletion src/Dropdown.php
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,9 @@ public static function show($itemtype, $options = [])
$params['entity'] = getSonsOf('glpi_entities', $params['entity']);
}
}
$params['entity'] = Session::getMatchingActiveEntities($params['entity']);
if ($params['entity'] !== null) {
$params['entity'] = Session::getMatchingActiveEntities($params['entity']);
}

$field_id = Html::cleanId("dropdown_" . $params['name'] . $params['rand']);

Expand Down
15 changes: 11 additions & 4 deletions src/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -1978,18 +1978,25 @@ public static function getMatchingActiveEntities(/*int|array*/ $entities_ids)/*:
return $entities_ids;
}

if (!is_array($entities_ids) && !is_int($entities_ids) && !ctype_digit($entities_ids)) {
if (
!is_array($entities_ids)
&& !is_int($entities_ids)
&& (!is_string($entities_ids) || !ctype_digit($entities_ids))
) {
// Unexpected value type.
return [];
}

$active_entities_ids = [];
foreach ($_SESSION['glpiactiveentities'] ?? [] as $active_entity_id) {
if (!is_int($active_entity_id) && !ctype_digit($active_entity_id)) {
if (
!is_int($active_entity_id)
&& (!is_string($active_entity_id) || !ctype_digit($active_entity_id))
) {
// Ensure no unexpected value converted to int
// as it would be converted to `0` and would permit access to root entity
trigger_error(
sprintf('Unexpected value `%s` found in `$_SESSION[\'glpiactiveentities\']`.', $active_entity_id),
sprintf('Unexpected value `%s` found in `$_SESSION[\'glpiactiveentities\']`.', $active_entity_id ?? 'null'),
E_USER_WARNING
);
continue;
Expand All @@ -2004,7 +2011,7 @@ public static function getMatchingActiveEntities(/*int|array*/ $entities_ids)/*:
$filtered = [];
foreach ((array)$entities_ids as $entity_id) {
if (
(is_int($entity_id) || ctype_digit($entity_id))
(is_int($entity_id) || (is_string($entity_id) && ctype_digit($entity_id)))
&& in_array((int)$entity_id, $active_entities_ids, true)
) {
$filtered[] = (int)$entity_id;
Expand Down
20 changes: 18 additions & 2 deletions tests/functional/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -722,20 +722,32 @@ protected function entitiesRestrictProvider(): iterable
'result' => is_array($entity_restrict) ? [2] : 2,
];
}

// Invalid null values in input
yield [
'entity_restrict' => null,
'active_entities' => [0, 1, '2', 3],
'result' => [],
];
yield [
'entity_restrict' => [1, null, 3],
'active_entities' => [0, 1, '2', 3],
'result' => [1, 3],
];
}

/**
* @dataProvider entitiesRestrictProvider
*/
public function testGetMatchingActiveEntities(/*int|array*/ $entity_restrict, ?array $active_entities, /*int|array*/ $result): void
public function testGetMatchingActiveEntities(/*mixed*/ $entity_restrict, ?array $active_entities, /*int|array*/ $result): void
{
$_SESSION['glpiactiveentities'] = $active_entities;
$this->variable(\Session::getMatchingActiveEntities($entity_restrict))->isIdenticalTo($result);
}

public function testGetMatchingActiveEntitiesWithUnexpectedValue(): void
{
$_SESSION['glpiactiveentities'] = [0, 1, 2, 'foo', 3];
$_SESSION['glpiactiveentities'] = [0, 1, 2, 'foo', null, 3];

$this->when(
function () {
Expand All @@ -744,6 +756,10 @@ function () {
)->error
->withType(E_USER_WARNING)
->withMessage('Unexpected value `foo` found in `$_SESSION[\'glpiactiveentities\']`.')
->exists()
->error
->withType(E_USER_WARNING)
->withMessage('Unexpected value `null` found in `$_SESSION[\'glpiactiveentities\']`.')
->exists();
}
}

0 comments on commit f1d2074

Please sign in to comment.