fix(nag): array offset access on null alarm/method values

[TDannhauer]

Fix PHP 8+ null array access in Nag alarm and notification form types

Summary

Nag_Form_Type_NagAlarm and Nag_Form_Type_NagMethod no longer assume alarm / methods values are always arrays. They now handle null (new tasks), integer alarm minutes (edit tasks loaded from toHash()), and submitted form arrays consistently in getInfo() and isValid().

This removes recurring log noise:

HORDE [nag] PHP ERROR: Trying to access array offset on null
  ... NagAlarm.php (lines 15, 31)
  ... NagMethod.php (line 43)

Problem

Symptom

When opening or validating the task form (especially New Task), Nag logs PHP errors about accessing array offsets on null.

Root cause

Nag uses two representations for alarms:

Layer	alarm	methods
Storage / Nag_Task::toHash()	int minutes (0 = off)	array or empty
HTML form (alarm[on], etc.)	['on' => …, 'value' => …, 'unit' => …]	['on' => …]

Nag_Ui_VarRenderer_Html::_renderVarInput_NagAlarm() already normalizes integers and null into the form array shape for display. The form type classes (getInfo(), isValid()) did not, and accessed $info['on'] / $value['on'] unconditionally.

Horde_Form_Variable::getValue() only applies setDefault() when the variable name is missing from vars. On edit, alarm is present as an integer from toHash(), so defaults never run. On new tasks, alarm / methods may be unset → null.

Affected code paths

• Form validation (isValid) on save or reload (actionID=task_form)
• getInfo() when building task properties from submitted vars
• Any code path that touches these types before POST data exists

Solution

lib/Form/Type/NagAlarm.php

getInfo()

• If value is not an array: return (int) $info when truthy (stored minutes), else 0.
• If array with empty on: return 0 (replaces assigning 0 to $info after reading ['on'] on a non-array).
• Otherwise unchanged: compute value * unit for enabled alarms.

isValid()

• Only enforce “due date required for alarm” when $value is an array with non-empty on.

lib/Form/Type/NagMethod.php

getInfo()

• Guard with !is_array($info) before empty($info['on']).

isValid()

• Treat $value as custom notification only when it is an array with on set.
• Derive whether alarm is on from either form array ($alarm['on']) or stored integer ($alarm minutes).

Code change (conceptual)

NagAlarm::getInfo() — before:

$info = $var->getValue($vars);
if (!$info['on']) {
    $info = 0;
} else {
    // ...
}

After:

$info = $var->getValue($vars);
if (!is_array($info)) {
    return $info ? (int) $info : 0;
}
if (empty($info['on'])) {
    return 0;
}
// ...

NagMethod::isValid() — before:

if ($value['on'] && !$alarm['on']) {

After:

$alarmOn = is_array($alarm) ? !empty($alarm['on']) : !empty($alarm);
if (is_array($value) && !empty($value['on']) && !$alarmOn) {

Files changed

File	Change
lib/Form/Type/NagAlarm.php	Null-safe getInfo(); null-safe isValid()
lib/Form/Type/NagMethod.php	Null-safe getInfo(); alarm-on check supports int or array in isValid()

Test plan

• Reload PHP-FPM / clear opcache after deploy.
• New task: Open add-task form; confirm no array offset on null errors in Nag logs.
• New task save: Save with alarm off, alarm on + due date, alarm on without due date (expect validation message, no PHP error).
• Edit task: Open task with existing alarm (integer minutes in DB); form renders; save without changes.
• Edit task: Toggle alarm on/off and custom notification method; save; verify task_alarm and task_alarm_methods in storage.
• Form reload: Trigger methods reload (notification field action); no PHP errors.
• Regression: tasks with alarm 0 and empty methods still save correctly.

Compatibility

• API / storage: Unchanged — getInfo() still returns alarm as integer minutes and methods as array (or []).
• UI: Unchanged — HTML renderer still owns display normalization; behavior matches prior intent.
• Breaking: None.

Follow-up (optional, not in this PR)

Normalization is still duplicated between Nag_Ui_VarRenderer_Html and these form types. A later refactor could:

• Add toFormValue() / toStorageValue() (or normalizeFormValue()) on the type classes.
• Call them from the renderer, getInfo(), isValid(), and optionally normalize vars when building the edit form from toHash().
• Set form defaults (['on' => false]) for new tasks.

That would standardize the form representation without changing Horde-wide storage format.

Related

• Package: horde/nag (branch dev-refactor/multi_tasklist_usage_2 in this deployment)
• Similar PHP 8 pattern: Nag_Form_Type_NagDue already casts due_type for null safety

[ralflang]

The underlying issue is fishy though.

[TDannhauer]

yes, i had the same conversation with AI, but fixing that we slip into the FormV3 migration.

I suggest we clean it further up after/while migration to Form V3