Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When using automation, numeric values may be treated as strings #4744

Closed
A200K opened this issue Apr 28, 2022 · 2 comments
Closed

When using automation, numeric values may be treated as strings #4744

A200K opened this issue Apr 28, 2022 · 2 comments
Labels
bug Undesired behaviour unverified Some days we don't have a clue
Milestone

Comments

@A200K
Copy link
Contributor

A200K commented Apr 28, 2022

Hey,

I recently discovered a bug in the automation rules and I'm surprised noone mentioned this yet.
The operators in the automatic graph creation criteria, e.g. 'greater than', 'less than or equal' etc. are always performing alphabetical instead of numerical comparison due to forced escaping in the sql query.
This might be desireable in some cases but it caused me some issues, as it the following example statements evaluate to true:
'200' < '3'
'50' >= '1000000'

Steps to reproduce the behavior:

  1. Create a graph rule, add graph creation criteria
  2. Set operator to 'is less than or equal'
  3. Select a field with a known value, that value might be 900
  4. Set 'Matching Pattern' to 1000
  5. See that it is not finding your device entry with value 900

I suggest this fix:
Original file: api/api_automation.php, function build_rule_item_filter, line ~1721-1724:

$sql_filter .= ' ' . $automation_op_array['op'][$automation_rule_item['operator']] . ' ';

if ($automation_op_array['binary'][$automation_rule_item['operator']]) {
	$sql_filter .= (db_qstr($automation_op_array['pre'][$automation_rule_item['operator']] . $automation_rule_item['pattern'] . $automation_op_array['post'][$automation_rule_item['operator']]));
}

To disable escaping numerical values:

$sql_filter .= ' ' . $automation_op_array['op'][$automation_rule_item['operator']] . ' ';

if ($automation_op_array['binary'][$automation_rule_item['operator']]) {
	// Fix:
	$query_value = $automation_op_array['pre'][$automation_rule_item['operator']] . $automation_rule_item['pattern'] . $automation_op_array['post'][$automation_rule_item['operator']];
	// Apply unescaped query to numeric values and numeric comparison operators
	if( is_numeric($query_value) && $automation_rule_item['operator'] >= AUTOMATION_OP_LT && $automation_rule_item['operator'] <= AUTOMATION_OP_GE )
		$sql_filter .= $query_value;
	else
		$sql_filter .= db_qstr($query_value);
}

If the matching sequence is not numeric, the old alphabetical style comparison will still be used.

@A200K A200K added bug Undesired behaviour unverified Some days we don't have a clue labels Apr 28, 2022
@TheWitness
Copy link
Member

Can you create a pull request please?

@A200K
Copy link
Contributor Author

A200K commented Apr 28, 2022

Can you create a pull request please?

sure thing

A200K pushed a commit to A200K/cacti that referenced this issue Apr 28, 2022
@A200K A200K closed this as completed May 1, 2022
@TheWitness TheWitness added this to the v1.2.21 milestone May 1, 2022
@netniV netniV changed the title Automation Rule Operators When using automation, numeric values may be treated as strings May 15, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour unverified Some days we don't have a clue
Projects
None yet
Development

No branches or pull requests

2 participants