A......... [ZBX-19461] fixed validation of the history functions second parameter

2 files changed, 19 insertions, 21 deletions

diff --git a/ui/include/classes/validators/CHistFunctionValidator.php b/ui/include/classes/validators/CHistFunctionValidator.php
index 3fee138cdf3..03d43159aa7 100644
--- a/ui/include/classes/validators/CHistFunctionValidator.php
+++ b/ui/include/classes/validators/CHistFunctionValidator.php
@@ -105,26 +105,6 @@ class CHistFunctionValidator extends CValidator {
 				continue;
 			}
 
-			switch ($param['type']) {
-				case CHistFunctionParser::PARAM_TYPE_PERIOD:
-					if (self::hasMacros($param['data']['sec_num']) && $param['data']['time_shift'] === '') {
-						continue 2;
-					}
-					break;
-
-				case CHistFunctionParser::PARAM_TYPE_QUOTED:
-					if (self::hasMacros(CHistFunctionParser::unquoteParam($param['match']))) {
-						continue 2;
-					}
-					break;
-
-				case CHistFunctionParser::PARAM_TYPE_UNQUOTED:
-					if (self::hasMacros($param['match'])) {
-						continue 2;
-					}
-					break;
-			}
-
 			if (array_key_exists('rules', $param_spec)) {
 				$is_valid = self::validateRules($param, $param_spec['rules'], $this->options);
 
@@ -187,6 +167,10 @@ class CHistFunctionValidator extends CValidator {
 						return false;
 					}
 
+					if (self::hasMacros($param['data']['sec_num']) && $param['data']['time_shift'] === '') {
+						return true;
+					}
+
 					if (!self::validatePeriod($param['data']['sec_num'], $param['data']['time_shift'], $rule['mode'])) {
 						return false;
 					}
@@ -194,6 +178,10 @@ class CHistFunctionValidator extends CValidator {
 					break;
 
 				case 'number':
+					if (self::hasMacros($param_match_unquoted)) {
+						return true;
+					}
+
 					$with_suffix = array_key_exists('with_suffix', $rule) && $rule['with_suffix'];
 
 					$parser = new CNumberParser(['with_minus' => true, 'with_suffix' => $with_suffix]);
@@ -212,13 +200,18 @@ class CHistFunctionValidator extends CValidator {
 					break;
 
 				case 'regexp':
-					if (preg_match($rule['pattern'], $param_match_unquoted) != 1) {
+					if (!self::hasMacros($param_match_unquoted)
+							&& preg_match($rule['pattern'], $param_match_unquoted) != 1) {
 						return false;
 					}
 
 					break;
 
 				case 'time':
+					if (self::hasMacros($param_match_unquoted)) {
+						return true;
+					}
+
 					$with_year = array_key_exists('with_year', $rule) && $rule['with_year'];
 					$min = array_key_exists('min', $rule) ? $rule['min'] : ZBX_MIN_INT32;
 					$max = array_key_exists('max', $rule) ? $rule['max'] : ZBX_MAX_INT32;
diff --git a/ui/tests/unit/include/classes/validators/CHistFunctionValidatorTest.php b/ui/tests/unit/include/classes/validators/CHistFunctionValidatorTest.php
index 141aeb6e4c8..abfb8118b26 100644
--- a/ui/tests/unit/include/classes/validators/CHistFunctionValidatorTest.php
+++ b/ui/tests/unit/include/classes/validators/CHistFunctionValidatorTest.php
@@ -45,6 +45,11 @@ class CHistFunctionValidatorTest extends TestCase {
 			['avg(/host/key,1s)', [], ['rc' => true, 'error' => null]],
 			['avg(/host/key, 1m)', [], ['rc' => true, 'error' => null]],
 			['avg(/host/key, 1M)', [], ['rc' => false, 'error' => 'invalid second parameter in function "avg"']],
+			['avg(/host/key, {$PERIOD})', [], ['rc' => true, 'error' => null]],
+			['avg(/host/key, {$PERIOD}:{$TIMESHIFT})', [], ['rc' => true, 'error' => null]],
+			['avg(/host/key, {$PERIOD}:now-5h)', [], ['rc' => true, 'error' => null]],
+			['avg(/host/key, {$PERIOD}:now/h-{$TIMESHIFT})', [], ['rc' => true, 'error' => null]],
+			['avg(/host/key, "{$PERIOD}")', [], ['rc' => false, 'error' => 'invalid second parameter in function "avg"']],
 			['avg(/host/key, 1m:now/h-1h)', [], ['rc' => true, 'error' => null]],
 			['avg(/host/key, #3:now/h-1h)', [], ['rc' => true, 'error' => null]],
 			['avg(/host/key, #5:now/M)', [], ['rc' => true, 'error' => null]],