Skip to content

fix: cleantemp cron doesn't remove tempnam()-created inventory temp files - #25100

Open
TarekRemo wants to merge 2 commits into
glpi-project:10.0/bugfixesfrom
TarekRemo:ticket_45694
Open

fix: cleantemp cron doesn't remove tempnam()-created inventory temp files#25100
TarekRemo wants to merge 2 commits into
glpi-project:10.0/bugfixesfrom
TarekRemo:ticket_45694

Conversation

@TarekRemo

Copy link
Copy Markdown
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • This change requires a documentation update.

Description

  • It fixes !45694
  • Since Fix inventory temp file not removed on import error #11550, Inventory::setData() writes the raw inventory payload with tempnam(GLPI_INVENTORY_DIR, 'xml_'|'json_'), producing extension-less filenames (xml_XXXXXXXX, json_XXXXXXXX) under _inventories/. The cleantemp CronTask (Inventory::cronCleantemp()) was never updated to match this new naming, so these files are silently never cleaned, even when the task is enabled.
  • Fix: added a second glob for the tempnam() naming pattern alongside the legacy one, and guards the removal with is_file().

@TarekRemo TarekRemo self-assigned this Aug 6, 2026
@trasher trasher added the bug label Aug 6, 2026
@trasher trasher modified the milestones: 11.0.9, 10.0.27 Aug 6, 2026
@trasher
trasher requested review from Rom1-B and trasher August 6, 2026 08:40

@trasher trasher left a comment

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.

lgtm

$temp_files = glob(GLPI_INVENTORY_DIR . '/*.{' . implode(',', $conf->knownInventoryExtensions()) . '}', GLOB_BRACE);

// Files created by `tempnam()` while an inventory is being processed (see `Inventory::setData()`).
$temp_files = array_merge($temp_files, glob(GLPI_INVENTORY_DIR . '/{xml_,json_}*', GLOB_BRACE));

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.

Can you add a test creating an old tempnam()-style file and asserting cronCleantemp() removes it?

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 August 6, 2026 14:23
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.

3 participants