Skip to content

[BUG] Option::getFromDB(1) called without checking return value — causes Undefined index errors #45

@giovanny07

Description

@giovanny07

Plugin version: 3.2.6
GLPI version: 11.x
Severity: High
Files: setup.php ~line 102, src/TicketTask.php, src/ProjectTask.php, src/PlanningExternalEvent.php, hook.php
Related issue: #23

Description

Throughout the plugin, the options record is loaded with:

$opt = new Option();
$opt->getFromDB(1);
// Direct field access immediately after, no existence check
$value = $opt->fields['use_timerepartition'];

CommonDBTM::getFromDB() returns false when the record does not exist, but its return value is never checked. When it returns false, $opt->fields remains an empty array, and every subsequent $opt->fields['any_key'] access generates an Undefined index / Undefined array key PHP notice or warning, which in GLPI 11 (strict mode) can escalate to a fatal error.

This happens in at least the following scenarios:

  • Fresh installation where plugin_activity_install() failed partway through (e.g., due to the typo in B2).
  • The glpi_plugin_activity_options table is empty for any reason.
  • A race condition during first install before INSERT into the options table completes.

Steps to reproduce

  1. Truncate glpi_plugin_activity_options (or install with a partial migration failure).
  2. Navigate to any page that loads the Activity plugin.
  3. Observe PHP notices/errors in GLPI logs: Undefined array key "use_timerepartition", Undefined array key "is_cra_default", etc.

Proposed fix

Option A — Guard every call site (minimal change)

$opt = new Option();
if (!$opt->getFromDB(1)) {
    // Record missing — insert defaults or skip optional behavior
    $opt->fields = $opt->getDefaultValues(); // see below
}

Option B — Add a getInstance() helper that guarantees a valid record (recommended)

Add a static method to Option that ensures the record exists before returning:

// src/Option.php

public static function getInstance(): self
{
    $opt = new self();
    if (!$opt->getFromDB(1)) {
        // Auto-create with default values on first call
        $defaults = self::getDefaultValues();
        $opt->add($defaults);
        $opt->getFromDB(1);
    }
    return $opt;
}

public static function getDefaultValues(): array
{
    return [
        'use_timerepartition'      => 0,
        'use_groupmanager'         => 0,
        'use_type_as_name'         => 0,
        'is_cra_default'           => 0,
        'use_mandaydisplay'        => 0,
        'use_project'              => 0,
        'use_weekend'              => 0,
        'use_hour_on_cra'          => 0,
        'show_planningevents_entity' => 0,
    ];
}

Then replace all $opt->getFromDB(1) call sites:

// ❌ Before (multiple files)
$opt = new Option();
$opt->getFromDB(1);
if ($opt->fields['use_timerepartition']) { ... }

// ✅ After
$opt = Option::getInstance();
if ($opt->fields['use_timerepartition']) { ... }

Also fix setup.php specifically

// setup.php — plugin_init_activity()

// ❌ Before
$opt = new Option();
$opt->getFromDB(1);

if (Plugin::isPluginActive("manageentities")
    || $opt->fields['use_timerepartition']) {

// ✅ After
$opt = Option::getInstance();

if (Plugin::isPluginActive("manageentities")
    || ($opt->fields['use_timerepartition'] ?? false)) {

Additional context

  • The ?? null-coalescing operator should be used as a safety net on all field accesses even after the guard, to prevent regressions if new fields are added without a corresponding migration.
  • This is likely the root cause of issue Undefined index - getFromDB(1)  #23 (Undefined index - getFromDB(1)).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions