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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions