Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Fixed

- Disable containers whose table name exceeds MySQL's 64-character limit instead of crashing; rename them from their edit form to reactivate, recovering existing data when a matching table is found.
- `plugins:fields:check_database` now also reports container/item type pairs with no matching table, and tables with no matching container/item type pair.

## [1.24.4] - 2026-08-06

### Fixed
Expand Down
4 changes: 4 additions & 0 deletions front/container.form.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@
$container->check($_POST['id'], UPDATE);
$container->update($_POST);
Html::back();
} elseif (isset($_POST['rename_oversized'])) {
$container->check($_POST['id'], UPDATE);
PluginFieldsContainer::renameOversizedContainer((int) $_POST['id'], (string) ($_POST['new_name'] ?? ''));
Html::back();
} elseif (isset($_POST['update_fields_values'])) {
$right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $_POST['plugin_fields_containers_id']);
if ($right > READ) {
Expand Down
71 changes: 50 additions & 21 deletions inc/checkdatabasecommand.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ protected function configure()
__('- some deleted fields may still be present in database (bug introduced in version %s and fixed in version %s)', 'fields'),
'1.15.0',
'1.15.3',
),
)
. "\n"
. __('- container/item type pairs with no matching table, or tables with no matching container/item type pair', 'fields'),
);

$this->addOption(
Expand All @@ -66,41 +68,68 @@ protected function execute(InputInterface $input, OutputInterface $output)
$dead_fields = PluginFieldsMigration::checkDeadFields($fix);
$dead_fields_count = count($dead_fields, COUNT_RECURSIVE) - count($dead_fields);

// No invalid fields found
if ($dead_fields_count === 0) {
$tables_consistency = PluginFieldsMigration::checkContainerTablesConsistency();

if ($dead_fields_count === 0 && $tables_consistency['missing'] === [] && $tables_consistency['orphaned'] === []) {
$output->writeln(
'<info>' . __('Everything is in order - no action needed.', 'fields') . '</info>',
);

return Command::SUCCESS;
}

// Indicate which fields will have been or must be deleted
$error = $fix
? sprintf(__('Database was containing orphaned data from %s improperly deleted field(s).', 'fields'), $dead_fields_count)
: sprintf(__('Database contains orphaned data from %s improperly deleted field(s).', 'fields'), $dead_fields_count);
$output->writeln('<error>' . $error . '</error>', OutputInterface::VERBOSITY_QUIET);
if ($dead_fields_count > 0) {
$error = $fix
? sprintf(__('Database was containing orphaned data from %s improperly deleted field(s).', 'fields'), $dead_fields_count)
: sprintf(__('Database contains orphaned data from %s improperly deleted field(s).', 'fields'), $dead_fields_count);
$output->writeln('<error>' . $error . '</error>', OutputInterface::VERBOSITY_QUIET);

foreach ($dead_fields as $table => $fields) {
foreach ($fields as $field) {
$info = $fix
? sprintf(__('-> "%s.%s" has been deleted.', 'fields'), $table, $field)
: sprintf(__('-> "%s.%s" should be deleted.', 'fields'), $table, $field);
$output->writeln($info);
foreach ($dead_fields as $table => $fields) {
foreach ($fields as $field) {
$info = $fix
? sprintf(__('-> "%s.%s" has been deleted.', 'fields'), $table, $field)
: sprintf(__('-> "%s.%s" should be deleted.', 'fields'), $table, $field);
$output->writeln($info);
}
}

// Show extra info in dry-run mode
if (!$fix) {
// Print command to do the actual deletion
$next_command = sprintf(
__('Run "%s" to fix database inconsistencies.', 'fields'),
sprintf('php bin/console %s --fix', $this->getName()),
);
$output->writeln(
'<comment>' . $next_command . '</comment>',
OutputInterface::VERBOSITY_QUIET,
);
}
}

// Show extra info in dry-run mode
if (!$fix) {
// Print command to do the actual deletion
$next_command = sprintf(
__('Run "%s" to fix database inconsistencies.', 'fields'),
sprintf('php bin/console %s --fix', $this->getName()),
if ($tables_consistency['missing'] !== []) {
$output->writeln(
'<error>' . sprintf(__('%d container/item type pair(s) have no matching table.', 'fields'), count($tables_consistency['missing'])) . '</error>',
OutputInterface::VERBOSITY_QUIET,
);
foreach ($tables_consistency['missing'] as $entry) {
$output->writeln(sprintf(
'-> container #%d (%s): expected table "%s" not found',
$entry['container_id'],
$entry['itemtype'],
$entry['table'],
));
Comment thread
Rom1-B marked this conversation as resolved.
}
}

if ($tables_consistency['orphaned'] !== []) {
$output->writeln(
'<comment>' . $next_command . '</comment>',
'<error>' . sprintf(__('%d table(s) do not match any container/item type pair.', 'fields'), count($tables_consistency['orphaned'])) . '</error>',
OutputInterface::VERBOSITY_QUIET,
);
foreach ($tables_consistency['orphaned'] as $table) {
$output->writeln(sprintf('-> "%s"', $table));
Comment thread
Rom1-B marked this conversation as resolved.
}
}

return Command::SUCCESS;
Expand Down
246 changes: 246 additions & 0 deletions inc/container.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,45 @@ public static function installUserData(Migration $migration, $version)
/** @var DBmysql $DB */
global $DB;

// Disable oversized-table containers instead of crashing.
$obj = new self();
$active_containers = $obj->find(['is_active' => 1]);
foreach ($active_containers as $container) {
if (empty($container['itemtypes'])) {
continue;
}

$itemtypes = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']);
if (!is_array($itemtypes)) {
continue;
}

$too_long = false;
foreach ($itemtypes as $itemtype) {
$table = getTableForItemType(self::getClassname($itemtype, $container['name']));
if (strlen($table) > 64) {
$too_long = true;
break;
}
}

if (!$too_long) {
continue;
}

$DB->update(
self::getTable(),
['is_active' => 0],
['id' => $container['id']],
);

$migration->addWarningMessage(sprintf(
__('Container #%1$d (%2$s) disabled: table name too long. Rename it from its edit form to reactivate it.', 'fields'),
$container['id'],
$container['name'],
));
}

// -> 0.90-1.3: generated class moved
// Drop them, they will be regenerated
$obj = new self();
Expand Down Expand Up @@ -443,6 +482,16 @@ public static function installUserData(Migration $migration, $version)
$obj = new self();
$containers = $obj->find();
foreach ($containers as $container) {
$itemtypes = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']);
if (is_array($itemtypes)) {
foreach ($itemtypes as $itemtype) {
if (strlen(getTableForItemType(self::getClassname($itemtype, $container['name']))) > 64) {
// Table name still too long: the container was just disabled above, skip it.
continue 2;
}
}
}

self::create($container);
}

Expand Down Expand Up @@ -928,6 +977,175 @@ public static function getTypeName($nb = 0)
return __('Block', 'fields');
}

/**
* Rename an oversized-table container, then reactivate it.
*
* @param int $id Container ID.
* @param string $new_name New internal name.
*/
public static function renameOversizedContainer(int $id, string $new_name): bool
{
/** @var DBmysql $DB */
global $DB;

$container = new self();
if (!$container->getFromDB($id)) {
Session::AddMessageAfterRedirect(sprintf(__('Unknown container #%d.', 'fields'), $id), false, ERROR);

return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renameOversizedContainer has no server-side guard that the target container is actually disabled or oversized.

Suggested change
if ((int) $container->fields['is_active'] !== 0) {
Session::AddMessageAfterRedirect(__('Container is already active.', 'fields'), false, ERROR);
return false;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, but a strict is_active === 0 check would break the orphan-recovery path (an active container whose table got corrupted by an old migration bug, see testRenameOversizedContainerRecoversDataFromOrphanTable). Added a guard instead that requires the container's current name to actually produce an oversized table, same condition already used to show the "Rename and reactivate" button.

$new_name = preg_replace('/[^\da-zA-Z]/', '', $new_name) ?? '';
if ($new_name === '') {
Session::AddMessageAfterRedirect(__('Invalid name.', 'fields'), false, ERROR);

return false;
}

$itemtypes = PluginFieldsToolbox::decodeJSONItemtypes($container->fields['itemtypes']);
if (!is_array($itemtypes) || $itemtypes === []) {
Session::AddMessageAfterRedirect(__('No associated item type.', 'fields'), false, ERROR);

return false;
}

$too_long_tables = [];
foreach ($itemtypes as $itemtype) {
$table = getTableForItemType(self::getClassname($itemtype, $new_name));
if (strlen($table) > 64) {
$too_long_tables[] = $table;
}
}

if ($too_long_tables !== []) {
Session::AddMessageAfterRedirect(sprintf(
__('Still too long: %s.', 'fields'),
implode(', ', $too_long_tables),
), false, ERROR);

return false;
}

$found = $container->find(['name' => $new_name]);
foreach ($found as $other) {
if ((int) $other['id'] === $id) {
continue;
}

$other_itemtypes = PluginFieldsToolbox::decodeJSONItemtypes($other['itemtypes']);
if (is_array($other_itemtypes) && array_intersect($itemtypes, $other_itemtypes) !== []) {
Session::AddMessageAfterRedirect(__('Name already used for this item type.', 'fields'), false, ERROR);

return false;
}
}

$plugin = new Plugin();
$plugin->getFromDBbyDir('fields');

$migration = new Migration((string) ($plugin->fields['version'] ?? ''));

$old_name = $container->fields['name'];
$claimed_orphans = [];
$data_preserved = false;
foreach ($itemtypes as $itemtype) {
$new_table = getTableForItemType(self::getClassname($itemtype, $new_name));

$old_table = getTableForItemType(self::getClassname($itemtype, $old_name));
if (!$DB->tableExists($old_table)) {
$old_table = self::findOrphanTableForContainer($id, $claimed_orphans);
}

if ($old_table === null || !$DB->tableExists($old_table)) {
continue;
}

$claimed_orphans[] = $old_table;

if (countElementsInTable($old_table) > 0) {
$migration->renameTable($old_table, $new_table);
$data_preserved = true;
} else {
// Empty leftover table: drop it, a fresh one is created below.
$migration->dropTable($old_table);
}
}

$DB->clearSchemaCache();

$migration->executeMigration();

$container->update([
'id' => $id,
'name' => $new_name,
'is_active' => 1,
]);

$container->getFromDB($id);
self::create($container->fields);

$message = $data_preserved
? sprintf(__('Renamed to "%s" and reactivated, existing data preserved.', 'fields'), $new_name)
: sprintf(__('Renamed to "%s" and reactivated, no existing data found.', 'fields'), $new_name);
Session::AddMessageAfterRedirect($message, false, INFO);

return true;
}

/**
* Find an orphaned table belonging to this container.
*
* @param int $container_id Container ID.
* @param string[] $already_claimed Orphan tables already assigned to another itemtype in this call.
*/
private static function findOrphanTableForContainer(int $container_id, array $already_claimed): ?string
{
/** @var DBmysql $DB */
global $DB;

$orphaned = array_diff(
PluginFieldsMigration::checkContainerTablesConsistency()['orphaned'],
$already_claimed,
);

// Primary match: `plugin_fields_containers_id` is DEFAULTed to the container's own id
// at table creation time, a link that survives any later name corruption.
$by_default = [];
foreach ($orphaned as $table) {
foreach ($DB->listFields($table) as $column) {
if ($column['Field'] === 'plugin_fields_containers_id' && (int) $column['Default'] === $container_id) {
$by_default[] = $table;
break;
}
}
}

if (count($by_default) === 1) {
return $by_default[0];
}

// Fallback for tables predating that default: match by custom field columns.
$expected_columns = PluginFieldsMigration::getValidFieldsForContainer($container_id);
if ($expected_columns === []) {
return null;
}

sort($expected_columns);

$base_columns = ['id', 'items_id', 'itemtype', 'plugin_fields_containers_id', 'entities_id'];
$candidates = [];
foreach (array_diff($orphaned, $by_default) as $table) {
$columns = array_diff(array_column($DB->listFields($table), 'Field'), $base_columns);
sort($columns);

if ($columns === $expected_columns) {
$candidates[] = $table;
}
}

return count($candidates) === 1 ? $candidates[0] : null;
}

public function showForm($ID, $options = [])
{
/** @var array $CFG_GLPI */
Expand Down Expand Up @@ -1062,6 +1280,34 @@ public function showForm($ID, $options = [])
echo '</td>';
echo '</tr>';

if (!$this->isNewID($ID) && (int) $this->fields['is_active'] === 0) {
$oversized_table = null;
$itemtypes = PluginFieldsToolbox::decodeJSONItemtypes($this->fields['itemtypes']);
if (is_array($itemtypes)) {
foreach ($itemtypes as $itemtype) {
$table = getTableForItemType(self::getClassname($itemtype, $this->fields['name']));
if (strlen($table) > 64) {
$oversized_table = $table;
break;
}
}
}

if ($oversized_table !== null) {
echo '<tr class="tab_bg_2">';
echo '<td colspan="2">';
echo sprintf(__('Table name too long (%s):', 'fields'), $oversized_table);
echo '</td>';
echo '<td>';
echo Html::input('new_name', ['placeholder' => __('New name', 'fields')]);
echo '</td>';
echo '<td>';
echo '<button type="submit" name="rename_oversized" class="btn btn-primary">' . __('Rename and reactivate', 'fields') . '</button>';
echo '</td>';
echo '</tr>';
}
}

$this->showFormButtons($options);

return true;
Expand Down
Loading