Skip to content

fix(container.class): Prevent item creation with null value for mandatory fields#1229

Merged
Rom1-B merged 3 commits into
pluginsGLPI:mainfrom
TarekRemo:ticket_45283
Jul 22, 2026
Merged

fix(container.class): Prevent item creation with null value for mandatory fields#1229
Rom1-B merged 3 commits into
pluginsGLPI:mainfrom
TarekRemo:ticket_45283

Conversation

@TarekRemo

Copy link
Copy Markdown
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

  • It fixes !45283
  • Before this fix, in preItem(), the validation of required fields (validateValues()) was only called if populateData() did not return false. However, populateData() returns false when none of the block's fields are present in the input. Result: A creation that did not submit the fields (API/CLI, or a form without mapping) completely bypassed the check and created the element with a NULL value in a required field.
  • Fix: On creation only (isNewItem()), when populateData() returns false and the continer type is dom, validateValues() is still called on an empty dataset, which causes the missing required fields to fail validation and blocks creation.

@TarekRemo TarekRemo self-assigned this Jul 21, 2026
@TarekRemo TarekRemo added the bug label Jul 21, 2026
@TarekRemo
TarekRemo requested a review from Rom1-B July 21, 2026 13:19
Comment thread inc/container.class.php
Comment on lines +1973 to +1980
if ($item->isNewItem() && $loc_c->fields['type'] === 'dom') {
$data = ['plugin_fields_containers_id' => $c_id];
if (self::validateValues($data, $item::getType(), isset($_REQUEST['massiveaction'])) === false) {
$item->input = [];

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.

The minimal $data array built for this new validation path omits the status field. populateData()'s normal path always sets $data[$status_field_name] from $item->input/$item->fields (lines 2029-2034), and validateValues() resolves $status_value = $data[$status_field_name] ?? null to apply PluginFieldsStatusOverride mandatory/readonly relaxations (line 1673). Since this new branch never sets that key, any status override waiving a field's mandatory flag for a given creation status is silently ignored here, so a legitimate creation (API/CLI, dom container, status override in place) gets incorrectly blocked.

Suggested change
if ($item->isNewItem() && $loc_c->fields['type'] === 'dom') {
$data = ['plugin_fields_containers_id' => $c_id];
if (self::validateValues($data, $item::getType(), isset($_REQUEST['massiveaction'])) === false) {
$item->input = [];
return false;
}
}
if ($item->isNewItem() && $loc_c->fields['type'] === 'dom') {
$status_field_name = PluginFieldsStatusOverride::getStatusFieldName($item::getType());
$data = ['plugin_fields_containers_id' => $c_id];
if (array_key_exists($status_field_name, $item->input) && $item->input[$status_field_name] !== '') {
$data[$status_field_name] = (int) $item->input[$status_field_name];
} elseif (array_key_exists($status_field_name, $item->fields) && $item->fields[$status_field_name] !== '') {
$data[$status_field_name] = (int) $item->fields[$status_field_name];
}
if (self::validateValues($data, $item::getType(), isset($_REQUEST['massiveaction'])) === false) {
$item->input = [];
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.

done

@TarekRemo
TarekRemo requested a review from Rom1-B July 22, 2026 07:05
@Rom1-B
Rom1-B merged commit 48adbc7 into pluginsGLPI:main Jul 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants