Skip to content

Fix: search crash on duplicate field names #1222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Rom1-B merged 1 commit into from
Jul 9, 2026
Merged

Fix: search crash on duplicate field names #1222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [unreleased]

### Fixed

- Fix search crash when two containers share a dropdown field with the same name.

## [1.24.2] - 2026-06-30

Expand Down
89 changes: 35 additions & 54 deletions hook.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,23 +359,20 @@ function plugin_fields_addWhere($link, $nott, $itemtype, $ID, $val, $searchtype)
/** @var DBmysql $DB */
global $DB;

$searchopt = Search::getOptions($itemtype);
$table = $searchopt[$ID]['table'];
$field = $searchopt[$ID]['field'];
$pfields_type = $searchopt[$ID]['pfields_type'] ?? '';
$searchopt = Search::getOptions($itemtype);
$table = $searchopt[$ID]['table'];
$field = $searchopt[$ID]['field'];
$pfields_fields_id = $searchopt[$ID]['pfields_fields_id'] ?? 0;

// Identify the field by its id (unique), not by its name: several containers
// can define a field with the same name, which would make a name-based lookup ambiguous.
$field_field = new PluginFieldsField();
if (!$field_field->getFromDB($pfields_fields_id)) {
return null;
}

if (
$field_field->getFromDBByCrit(
[
'name' => $field,
'type' => 'number',
],
)
&& $pfields_type == 'number'
) {
// if 'number' field with name is found with searchtype 'equals' or 'notequals'
if ($field_field->fields['type'] === 'number') {
// if 'number' field with searchtype 'equals' or 'notequals'
// update WHERE clause with `$table_$field.$field` because without `$table_$field.id` is used
if ($searchtype == 'equals' || $searchtype == 'notequals') {
$operator = ($searchtype == 'equals') ? '=' : '!=';
Expand All @@ -385,7 +382,7 @@ function plugin_fields_addWhere($link, $nott, $itemtype, $ID, $val, $searchtype)

return $link . ' CAST(' . $DB->quoteName($table . '_' . $field) . '.' . $DB->quoteName($field) . ' AS DECIMAL(10,7))' . $operator . ' ' . $DB->quoteValue($val);
} else {
// if 'number' field with name is found with <= or >= or < or > search
// if 'number' field with <= or >= or < or > search
// update WHERE clause with the correct operator
$val = html_entity_decode((string) $val);
if (preg_match('/(<=|>=|>|<)/', $val, $matches)) {
Expand All @@ -396,49 +393,33 @@ function plugin_fields_addWhere($link, $nott, $itemtype, $ID, $val, $searchtype)
}
}

// if 'multiple' field with name is found -> 'Dropdown-XXXX' case
if (!$field_field->fields['multiple']) {
return null;
}

// 'Dropdown-XXXX' case: the searchopt field name is the plugin field name itself
// update WHERE clause with LIKE statement
if (
$field_field->getFromDBByCrit(
[
'name' => $field,
'multiple' => true,
],
)
) {
if (preg_match('/^dropdown-.+$/i', (string) $field_field->fields['type'])) {
$tablefield = $table . '_' . $field;
switch ($searchtype) {
case 'equals':
return PluginFieldsDropdown::multipleDropdownAddWhere($link, $tablefield, $field, $val, $nott ? 'notequals' : 'equals', $field_field);
case 'notequals':
return PluginFieldsDropdown::multipleDropdownAddWhere($link, $tablefield, $field, $val, $nott ? 'equals' : 'notequals', $field_field);
}
} else {
// if 'multiple' field with cleaned name is found -> 'dropdown' case
// update WHERE clause with LIKE statement
$cleanfield = str_replace('plugin_fields_', '', $field);
$cleanfield = str_replace('dropdowns_id', '', $cleanfield);
$tablefield = $table . '_' . $cleanfield;
if (
$field_field->getFromDBByCrit(
[
'name' => $cleanfield,
'multiple' => true,
],
)
) {
switch ($searchtype) {
case 'equals':
return PluginFieldsDropdown::multipleDropdownAddWhere($link, $tablefield, $field, $val, $nott ? 'notequals' : 'equals', $field_field);
case 'notequals':
return PluginFieldsDropdown::multipleDropdownAddWhere($link, $tablefield, $field, $val, $nott ? 'equals' : 'notequals', $field_field);
}
} else {
return false;
}
return match ($searchtype) {
'equals' => PluginFieldsDropdown::multipleDropdownAddWhere($link, $tablefield, $field, $val, $nott ? 'notequals' : 'equals', $field_field),
'notequals' => PluginFieldsDropdown::multipleDropdownAddWhere($link, $tablefield, $field, $val, $nott ? 'equals' : 'notequals', $field_field),
default => null,
};
}

return null;
// 'dropdown' case: the searchopt field name is mangled ("plugin_fields_<name>dropdowns_id"),
// recover the real field name to build the joined table alias
// update WHERE clause with LIKE statement
$cleanfield = str_replace('plugin_fields_', '', $field);
$cleanfield = str_replace('dropdowns_id', '', $cleanfield);

$tablefield = $table . '_' . $cleanfield;
return match ($searchtype) {
'equals' => PluginFieldsDropdown::multipleDropdownAddWhere($link, $tablefield, $field, $val, $nott ? 'notequals' : 'equals', $field_field),
'notequals' => PluginFieldsDropdown::multipleDropdownAddWhere($link, $tablefield, $field, $val, $nott ? 'equals' : 'notequals', $field_field),
default => null,
};
}

function plugin_item_transfer_fields(array $options): void
Expand Down
128 changes: 128 additions & 0 deletions tests/Units/HookAddWhereTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
<?php

/**
* -------------------------------------------------------------------------
* Fields plugin for GLPI
* -------------------------------------------------------------------------
*
* LICENSE
*
* This file is part of Fields.
*
* Fields is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* Fields is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Fields. If not, see <http://www.gnu.org/licenses/>.
* -------------------------------------------------------------------------
* @copyright Copyright (C) 2013-2023 by Fields plugin team.
* @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html
* @link https://github.com/pluginsGLPI/fields
* -------------------------------------------------------------------------
*/

namespace GlpiPlugin\Field\Tests\Units;

use Computer;
use Glpi\Tests\DbTestCase;
use Glpi\Tests\GLPITestCase;
use GlpiPlugin\Field\Tests\FieldTestTrait;
use PluginFieldsContainer;
use PluginFieldsField;
use Search;

require_once __DIR__ . '/../FieldTestCase.php';

/**
* Regression test for ticket #45192: a search on an itemtype threw
* TooManyResultsException as soon as two containers defined a 'dropdown'
* field with the same name.
*
* PluginFieldsField::prepareName() intentionally links a new 'dropdown'
* field to an existing field with the same name so both containers share
* the same underlying dropdown table. This produces two distinct rows in
* glpi_plugin_fields_fields with the same name, and plugin_fields_addWhere()
* used to look the field up by name, which is ambiguous in that case.
*/
final class HookAddWhereTest extends DbTestCase
{
use FieldTestTrait;

public function setUp(): void
{
GLPITestCase::setUp();
}

public function tearDown(): void
{
$this->tearDownFieldTest();
GLPITestCase::tearDown();
}

public function testAddWhereDoesNotThrowOnDuplicateFieldName(): void
{
$this->login();

$container1 = $this->createFieldContainer([
'label' => 'Container A ' . $this->getUniqueString(),
'type' => 'tab',
'itemtypes' => [Computer::class],
'is_active' => 1,
'entities_id' => 0,
'is_recursive' => 1,
]);

// Bypass createField(): default_value gets JSON re-encoded by prepareInputForAdd()
// and cannot be compared as-is, so it must be excluded from the input check.
$field1 = $this->createItem(PluginFieldsField::class, [
'label' => 'Accessoire',
'type' => 'dropdown',
'multiple' => 1,
'default_value' => [],
PluginFieldsContainer::getForeignKeyField() => $container1->getID(),
'ranking' => 1,
'is_active' => 1,
'is_readonly' => 0,
], ['allowed_values', 'question_types', 'default_value']);

$container2 = $this->createFieldContainer([
'label' => 'Container B ' . $this->getUniqueString(),
'type' => 'tab',
'itemtypes' => [Computer::class],
'is_active' => 1,
'entities_id' => 0,
'is_recursive' => 1,
]);

$field2 = $this->createItem(PluginFieldsField::class, [
'label' => 'Accessoire',
'type' => 'dropdown',
'multiple' => 1,
'default_value' => [],
PluginFieldsContainer::getForeignKeyField() => $container2->getID(),
'ranking' => 1,
'is_active' => 1,
'is_readonly' => 0,
], ['allowed_values', 'question_types', 'default_value']);

// Both containers intentionally share the same underlying field name.
$this->assertSame($field1->fields['name'], $field2->fields['name']);

$searchopt = Search::getOptions(Computer::class);
$so_id = PluginFieldsField::SEARCH_OPTION_STARTING_INDEX + $field1->getID();
$this->assertArrayHasKey($so_id, $searchopt);

// Before the fix, this threw:
// `PluginFieldsField::getFromDBByCrit()` expects to get one result, 2 found.
$result = plugin_fields_addWhere('', false, Computer::class, $so_id, '1', 'equals');

$this->assertNotFalse($result);
}
}