bugfix: trim xml import payload value before emptiness check

CHANGELOG.md

@@ -3,6 +3,7 @@
 --- develop ---
 
 * issue#250: Fix date filter persistence by validating before shift_span detection
+* issue#269: Correct XML import payload emptiness check to trim the request value
 * issue: Making changes to support Cacti 1.3
 * issue: Don't use MyISAM for non-analytical tables
 * issue: The install advisor for Syslog was broken in current Cacti releases

syslog_alerts.php

@@ -939,9 +939,11 @@ function import() {
 }
 
 function alert_import() {
-	if (trim(get_nfilter_request_var('import_text') != '')) {
+	$import_text = get_nfilter_request_var('import_text');
+
+	if (trim($import_text) != '') {
 		/* textbox input */
-		$xml_data = get_nfilter_request_var('import_text');
+		$xml_data = $import_text;
 	} elseif (($_FILES['import_file']['tmp_name'] != 'none') && ($_FILES['import_file']['tmp_name'] != '')) {
 		/* file upload */
 		$fp = fopen($_FILES['import_file']['tmp_name'],'r');
@@ -1009,4 +1011,3 @@ function alert_import() {
 
 	header('Location: syslog_alerts.php');
 }
-

syslog_removal.php

@@ -739,9 +739,11 @@ function import() {
 }
 
 function removal_import() {
-	if (trim(get_nfilter_request_var('import_text') != '')) {
+	$import_text = get_nfilter_request_var('import_text');
+
+	if (trim($import_text) != '') {
 		/* textbox input */
-		$xml_data = get_nfilter_request_var('import_text');
+		$xml_data = $import_text;
 	} elseif (($_FILES['import_file']['tmp_name'] != 'none') && ($_FILES['import_file']['tmp_name'] != '')) {
 		/* file upload */
 		$fp = fopen($_FILES['import_file']['tmp_name'],'r');
@@ -810,4 +812,3 @@ function removal_import() {
 
 	header('Location: syslog_removal.php');
 }
-

syslog_reports.php

@@ -801,9 +801,11 @@ function import() {
 }
 
 function report_import() {
-	if (trim(get_nfilter_request_var('import_text') != '')) {
+	$import_text = get_nfilter_request_var('import_text');
+
+	if (trim($import_text) != '') {
 		/* textbox input */
-		$xml_data = get_nfilter_request_var('import_text');
+		$xml_data = $import_text;
 	} elseif (($_FILES['import_file']['tmp_name'] != 'none') && ($_FILES['import_file']['tmp_name'] != '')) {
 		/* file upload */
 		$fp = fopen($_FILES['import_file']['tmp_name'],'r');
@@ -872,4 +874,3 @@ function report_import() {
 
 	header('Location: syslog_reports.php');
 }
-

tests/regression/issue269_import_text_branch_logic_test.php

@@ -0,0 +1,68 @@
+<?php
+
+/*
+ * Regression test for issue #269 -- branch-logic invariants.
+ *
+ * These assertions verify the structural properties that make whitespace-only
+ * input fall through to the file-upload branch instead of the textbox branch,
+ * and that a non-empty payload is assigned to $xml_data without further
+ * modification.  Pure source inspection: the functions themselves cannot be
+ * called in isolation because they depend on the Cacti runtime.
+ */
+
+$root = dirname(__DIR__, 2);
+$targets = array(
+	'alert_import'   => $root . '/syslog_alerts.php',
+	'removal_import' => $root . '/syslog_removal.php',
+	'report_import'  => $root . '/syslog_reports.php',
+);
+
+foreach ($targets as $func => $target) {
+	$content = file_get_contents($target);
+
+	if ($content === false) {
+		fwrite(STDERR, "Failed to load $target\n");
+		exit(1);
+	}
+
+	/*
+	 * 1. The request variable must be captured into a local first.
+	 *    Whitespace-only input falls through only because trim() is applied
+	 *    to the local; if the assignment were missing the condition would
+	 *    be wrong.
+	 */
+	if (!preg_match('/\$import_text\s*=\s*get_nfilter_request_var\s*\(\s*\'import_text\'\s*\)/', $content)) {
+		fwrite(STDERR, "$func: \$import_text assignment via get_nfilter_request_var missing in $target\n");
+		exit(1);
+	}
+
+	/*
+	 * 2. The branch condition must trim the local variable, not the raw
+	 *    request call.  This is what makes whitespace-only values fall
+	 *    through to the file-upload branch.
+	 */
+	if (!preg_match('/trim\s*\(\s*\$import_text\s*\)\s*!=\s*\'\'/', $content)) {
+		fwrite(STDERR, "$func: trim(\$import_text) != '' condition missing in $target\n");
+		exit(1);
+	}
+
+	/*
+	 * 3. Inside the textbox branch, $xml_data must be assigned the
+	 *    untrimmed local.  A non-empty payload is preserved as-is.
+	 */
+	if (!preg_match('/\$xml_data\s*=\s*\$import_text\s*;/', $content)) {
+		fwrite(STDERR, "$func: \$xml_data = \$import_text assignment missing in $target\n");
+		exit(1);
+	}
+
+	/*
+	 * 4. The file-upload branch must still exist (elseif on $_FILES).
+	 *    Ensures the fallback path was not accidentally removed.
+	 */
+	if (!preg_match('/elseif\s*\(\s*\(\s*\$_FILES\s*\[/', $content)) {
+		fwrite(STDERR, "$func: \$_FILES elseif branch missing in $target\n");
+		exit(1);
+	}
+}
+
+echo "issue269_import_text_branch_logic_test passed\n";

tests/regression/issue269_import_text_trim_check_test.php

@@ -0,0 +1,45 @@
+<?php
+
+$root = dirname(__DIR__, 2);
+$targets = array(
+	$root . '/syslog_alerts.php',
+	$root . '/syslog_reports.php',
+	$root . '/syslog_removal.php'
+);
+
+$legacy = "trim(get_nfilter_request_var('import_text') != '')";
+
+foreach ($targets as $target) {
+	$content = file_get_contents($target);
+
+	if ($content === false) {
+		fwrite(STDERR, "Failed to load $target\n");
+		exit(1);
+	}
+
+	if (strpos($content, $legacy) !== false) {
+		fwrite(STDERR, "Legacy import_text trim/comparison bug remains in $target\n");
+		exit(1);
+	}
+
+	$fixedPattern = '/trim\s*\(\s*\$import_text\s*\)\s*!=\s*\'\'/';
+	if (!preg_match($fixedPattern, $content)) {
+		fwrite(STDERR, "Fixed import_text trim/comparison check missing in $target\n");
+		exit(1);
+	}
+
+	/* After the local $import_text assignment, there must be no second
+	   get_nfilter_request_var('import_text') call.  A duplicate call
+	   would bypass the cached local variable. */
+	$needle = "\$import_text = get_nfilter_request_var('import_text')";
+	$assignPos = strpos($content, $needle);
+	if ($assignPos !== false) {
+		$afterAssign = substr($content, $assignPos + strlen($needle));
+		if (preg_match('/get_nfilter_request_var\s*\(\s*\'import_text\'\s*\)/', $afterAssign)) {
+			fwrite(STDERR, "Redundant get_nfilter_request_var('import_text') call after local assignment in $target\n");
+			exit(1);
+		}
+	}
+}
+
+echo "issue269_import_text_trim_check_test passed\n";