Add safety guards in Generate item flow for invalid references and non-existent classes#578
Open
bygadd wants to merge 1 commit into
Open
Conversation
…n-existent classes
The Generate item massive action threw fatal errors on GLPI 11 / PHP 8.3
under two scenarios:
1. PluginOrderReference::getReferenceItemID() (reference.class.php)
- getItemForItemtype() returns false in HTTP context for some
dynamically-generated classes (e.g. Glpi\CustomAsset\* in GLPI 11),
causing "Call to a member function getFromDB() on false".
- Existing references migrated from older itemtypes may have
templates_id = 0, which makes getFromDB() fail silently and the
subsequent getField() calls operate on a half-initialised object.
2. PluginOrderLink::generateItem() (link.class.php)
- Toolbox::hasTrait() internally calls class_uses(). On PHP 8.3 with
the Safe library wrapper, class_uses() on a non-existent class
throws Safe\Exceptions\SplException instead of emitting a warning.
- This is triggered by stale itemtype values in
glpi_plugin_order_orders_items that reference classes from
uninstalled plugins (e.g. PluginGenericobjectOther after migrating
to native custom assets).
Both call sites are now guarded:
- reference.class.php: bail out early with return 0 when the item
cannot be instantiated or templates_id is empty.
- link.class.php: prefix both Toolbox::hasTrait() calls with
class_exists() so non-existent classes are skipped cleanly.
Tested on GLPI 11.0.7, PHP 8.3, Order plugin 2.12.6:
- Order with valid native itemtypes (Computer, Monitor, etc.): unchanged.
- Order containing custom asset references (Glpi\CustomAsset\otherAsset):
Generate item form now renders fully and submits successfully.
- Order containing stale itemtypes from uninstalled plugins
(PluginGenericobjectOther): items are skipped without fatal error,
remaining items render correctly.
4 tasks
Author
|
Related: opened pluginsGLPI/room#64 today documenting similar GLPI 11 compatibility issues in the room plugin — including null-safety patterns analogous to the ones in this PR (guard before object instantiation, guard before trait checks). The room patch set is in production use on the same instance where this PR's guards were developed (GLPI 11.0.7, ~700K warnings/day → 0). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Generate item massive action throws fatal errors on GLPI 11 / PHP 8.3 under two scenarios that have become common after the GLPI 10 → 11 migration:
Invalid item references in
PluginOrderReference::getReferenceItemID()—getItemForItemtype()returnsfalsein HTTP context for some dynamically-generated classes (notablyGlpi\CustomAsset\*in GLPI 11), causingCall to a member function getFromDB() on false. Existing references migrated from older itemtypes may also havetemplates_id = 0, which makesgetFromDB()fail silently and downstreamgetField()calls operate on a half-initialised object.Non-existent classes in
PluginOrderLink::generateItem()—Toolbox::hasTrait()internally callsclass_uses(). On PHP 8.3 with the Safe library wrapper,class_uses()on a non-existent class throwsSafe\Exceptions\SplExceptioninstead of emitting a warning. This is triggered by stale itemtype values inglpi_plugin_order_orders_itemsthat reference classes from uninstalled plugins (e.g.PluginGenericobjectOtherafter migrating to native custom assets via the GenericObject 3.0.0 EOL migration tool).Implementation
Both call sites are now guarded:
inc/reference.class.php— bail out early withreturn 0when the item cannot be instantiated ortemplates_idis empty.inc/link.class.php— prefix bothToolbox::hasTrait()calls withclass_exists()so non-existent classes are skipped cleanly, both in the iteration loop (where$row['assignableitem']is set) and in the finalTemplateRenderer::display()call.The changes are defensive and minimal — they preserve all existing behaviour for valid native itemtypes, and only short-circuit when the underlying data cannot be resolved to a working class.
Testing
Tested on GLPI 11.0.7, PHP 8.3, Order plugin 2.12.6:
Glpi\CustomAsset\otherAsset) — form now renders fully and submitsPluginGenericobjectOther) — items skipped without fatal error, remaining items renderTested on a production instance with ~21,000 order line items and ~1,300 references, including a mix of native itemtypes, custom asset references, and stale references from a deinstalled plugin.
Checklist