Skip to content

Substitution: thold_str_replace blanks the value zero, get_current_value falls back to the first data source #787

Description

@somethingwithproof

Two substitution helpers return the wrong value for inputs that occur in normal operation.

thold_str_replace() blanks the value zero (thold_functions.php:8314-8320)

function thold_str_replace($search, $replace, $subject) {
    if (empty($replace) || $replace === 0) {
        $replace = '';
    }

    return str_replace($search, $replace, $subject);
}

empty('0') is true, so a replacement value of 0 or '0' becomes an empty string. Confirmed:

thold_str_replace('<X>', 0,   'v=<X>')  =>  'v='
thold_str_replace('<X>', '0', 'v=<X>')  =>  'v='
thold_str_replace('<X>', 5,   'v=<X>')  =>  'v=5'

This helper performs every tag substitution in thold_replace_threshold_tags() and the <SUBJECT> substitution in thold_mail(). So an alert for a value that dropped to zero reads "Current value is " with a blank — exactly the case the operator most needs to see.

The same blanking applies to trigger commands, where a script invoked as --value <CURRENTVALUE> receives a missing argument and its remaining arguments shift position.

Testing $replace === null || $replace === false instead would keep zero intact.

get_current_value() returns the wrong data source's value (thold_functions.php:5075-5081)

$idx = array_search($data_template_rrd_id, $result['data_source_names'], true);

// Return Blank if the value was not found (Cache Cleared?)

if (!isset($result['values']) || $idx === null || !cacti_sizeof($result['values'][$idx])) {
    return 0;
}

$value = array_values($result['values'][$idx])[0];

array_search() returns false when there is no match, never null, so the $idx === null guard never fires. $result['values'][false] is $result['values'][0] under PHP's bool-to-int key cast, so a lookup for a data source that does not exist quietly returns the first data source's value.

This is reachable today. thold_expression_specialtype_rpn() (:723-747) calls get_current_value() with 'upper_limit', 'lower_limit', 'rrd_minimum', 'rrd_maximum' and 'hdd_total' — none of which are data source names — as do the CDEF substitutions at :4817-4823. All of them resolve to the first data source's current value instead of the graph or data source limit that was asked for.

$idx === false fixes the guard. The call sites then need to look up the actual column rather than passing it as a data source name.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions