[icinga-checkins] icinga.org: icinga-core/mfriedrich/core: core: unknown macros are not replaced, and misleading to single dollar signs #2291 - MF

git at icinga.org git at icinga.org
Wed Aug 22 19:53:18 CEST 2012


Module: icinga-core
Branch: mfriedrich/core
Commit: 02341c6222e603d3357016a11714e28a92e33169
URL:    https://git.icinga.org/?p=icinga-core.git;a=commit;h=02341c6222e603d3357016a11714e28a92e33169

Author: Michael Friedrich <michael.friedrich at univie.ac.at>
Date:   Wed Aug 22 19:46:32 2012 +0200

core: unknown macros are not replaced, and misleading to single dollar signs #2291 - MF

people report bugs, that check_bla -c $foo$ will lead into errors, like
the shell interpreting $foo as variable, but $ throws an error then.
others tell to put quotes around, especially having a password.

the correct way of putting dollar signs into command lines is to
properly escape them with another dollar sign (see the docs). all other
methods are not valid, and only circumvent the real problem.

other than that, the config could contain macros which are not valid for
the object executing the command. those macros were not replaced either
and therefore causing the commands to fail, leading to the infamous bug
reports too.

the best way to enforce this being fixed is logging a warning instead of
just leaving that on the shell. default will now remove the unknown
macro string from the macro buffer, leading to safety.

since this will possibly break all other existing setups, having their
own tricks and workarounds applied, we'll add the new icinga.cfg option
keep_unknown_macros, which can be re-enabled in order to revert to the
old, buggy behaviour.

it's up to each user which to chose, in favor of resolving a long
lasting bug, the default will change with 1.8.

refs #2291

---

 Changelog                   |    6 ++++
 base/config.c               |   13 +++++++++
 base/icinga.c               |    2 +
 base/utils.c                |    3 ++
 common/macros.c             |   20 ++++++++++---
 sample-config/icinga.cfg.in |   15 ++++++++++
 tests/etc/2291.cfg          |   64 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 118 insertions(+), 5 deletions(-)

diff --git a/Changelog b/Changelog
index 5583880..bc435f8 100644
--- a/Changelog
+++ b/Changelog
@@ -69,6 +69,8 @@ FIXES
 * ocre: remove weird switch() statement when scanning checkresult queue (Andreas Ericsson) #2950 - MF
 * core: fix deleting too old check result files (Andreas Ericsson) #2951 - MF
 * core: avoid duplicate events when scheduling forced host|service check (Imri Zvik) #2993
+* core: unknown macros are not replaced, and misleading to single dollar signs #2291 - MF
+	** you can revert to the old behaviour with keep_unknown_macros=1 in icinga.cfg
 
 * classic ui: fix setting send_notification or sticky_ack as GET param has no effect on cmd.cgi acks #2926 - MF
 	** now you can finally disable checkboxes default ticked with these options
@@ -84,6 +86,10 @@ CHANGES
 * core: new command
 	** DISABLE_NOTIFICATIONS_EXPIRE_TIME;<schedule_time>;<expire_time> (set schedule_time to now()) #905
 
+* core: unknown macros are not left on the output anymore, logging a warning instead
+	** either fix your config (i.e. wrong macros, escape dollar signs with another one)
+	** or set keep_unknown_macros=1 in icinga.cfg to revert to the old behaviour
+
 * icinga.spec: add devel package #2634
 * icinga.spec: forced update on icinga.cfg change package locations #2923
 
diff --git a/base/config.c b/base/config.c
index 972e25b..f8a8e36 100644
--- a/base/config.c
+++ b/base/config.c
@@ -196,6 +196,8 @@ extern int      stalking_notifications_for_services;
 extern int      date_format;
 extern char     *use_timezone;
 
+extern int	keep_unknown_macros;
+
 extern contact		*contact_list;
 extern contactgroup	*contactgroup_list;
 extern host             *host_list;
@@ -1451,6 +1453,17 @@ int read_main_config_file(char *main_config_file) {
 			allow_empty_hostgroup_assignment = (atoi(value) > 0) ? TRUE : FALSE;
 		}
 
+                else if (!strcmp(variable, "keep_unknown_macros")) {
+
+                        if (strlen(value) != 1 || value[0] < '0' || value[0] > '1') {
+                                dummy = asprintf(&error_message, "Illegal value for check_service_freshness");
+                                error = TRUE;
+                                break;
+                        }
+
+                        keep_unknown_macros = (atoi(value) > 0) ? TRUE : FALSE;
+                }
+
 		/*** AUTH_FILE VARIABLE USED BY EMBEDDED PERL INTERPRETER ***/
 		else if (!strcmp(variable, "auth_file")) {
 
diff --git a/base/icinga.c b/base/icinga.c
index ee2076b..c2dc4b2 100644
--- a/base/icinga.c
+++ b/base/icinga.c
@@ -255,6 +255,8 @@ int             command_file_created = FALSE;
 int             event_profiling_enabled = FALSE;
 #endif
 
+int		keep_unknown_macros = FALSE;
+
 extern contact	       *contact_list;
 extern contactgroup    *contactgroup_list;
 extern hostgroup       *hostgroup_list;
diff --git a/base/utils.c b/base/utils.c
index abfbe04..ceabd50 100644
--- a/base/utils.c
+++ b/base/utils.c
@@ -236,6 +236,8 @@ extern int      stalking_notifications_for_services;
 
 extern int      date_format;
 
+extern int 	keep_unknown_macros;
+
 extern contact		*contact_list;
 extern contactgroup	*contactgroup_list;
 extern host             *host_list;
@@ -4486,6 +4488,7 @@ int reset_variables(void) {
 	stalking_notifications_for_hosts = DEFAULT_STALKING_NOTIFICATIONS_FOR_HOSTS;
 	stalking_notifications_for_services = DEFAULT_STALKING_NOTIFICATIONS_FOR_SERVICES;
 
+	keep_unknown_macros = FALSE;
 	external_command_buffer_slots = DEFAULT_EXTERNAL_COMMAND_BUFFER_SLOTS;
 
 	debug_level = DEFAULT_DEBUG_LEVEL;
diff --git a/common/macros.c b/common/macros.c
index 9e67759..6e5b6c9 100644
--- a/common/macros.c
+++ b/common/macros.c
@@ -38,6 +38,7 @@
 #ifdef NSCORE
 extern int      use_large_installation_tweaks;
 extern int      enable_environment_macros;
+extern int	keep_unknown_macros;
 #endif
 
 int dummy;	/* reduce compiler warnings */
@@ -226,12 +227,21 @@ int process_macros_r(icinga_macros *mac, char *input_buffer, char **output_buffe
 
 				log_debug_info(DEBUGL_MACROS, 2, "  Non-macro.  Running output (%lu): '%s'\n", (unsigned long)strlen(*output_buffer), *output_buffer);
 
-				/* add the plain text to the end of the already processed buffer */
-				*output_buffer = (char *)realloc(*output_buffer, strlen(*output_buffer) + strlen(temp_buffer) + 3);
-				strcat(*output_buffer, "$");
-				strcat(*output_buffer, temp_buffer);
-				if (buf_ptr != NULL)
+#ifdef NSCORE
+				if (keep_unknown_macros == TRUE) {
+#endif
+					/* add the plain text to the end of the already processed buffer */
+					*output_buffer = (char *)realloc(*output_buffer, strlen(*output_buffer) + strlen(temp_buffer) + 3);
 					strcat(*output_buffer, "$");
+					strcat(*output_buffer, temp_buffer);
+					if (buf_ptr != NULL)
+						strcat(*output_buffer, "$");
+#ifdef NSCORE
+				} else {
+					/* do not process unknown macros */
+					logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Skipping unknown macro '$%s$', removing it from output! Fix your config, or set keep_unknown_macros accordingly...\n", temp_buffer);
+				}
+#endif
 			}
 
 			/* insert macro */
diff --git a/sample-config/icinga.cfg.in b/sample-config/icinga.cfg.in
index 61b18ee..035b0f7 100644
--- a/sample-config/icinga.cfg.in
+++ b/sample-config/icinga.cfg.in
@@ -1319,6 +1319,21 @@ illegal_macro_output_chars=`~$&|'"<>
 
 
 
+# KEEP UNKNOWN MACROS
+# This option can be used to keep unknown macros within the output.
+# e.g. check_proc -C $foo$ will remain.
+# This was the default in versions older than Icinga 1.8, but now
+# the default is to remove those macros from the output, causing
+# the shell to interpret $foo and leaving a single $ there. See
+# #2291 for further information.
+# Make sure to escape single dollar signs with another '$', as the
+# docs describe. Other than that, enable this setting to revert to
+# the old behaviour.
+
+keep_unknown_macros=0
+
+
+
 # REGULAR EXPRESSION MATCHING
 # This option controls whether or not regular expression matching
 # takes place in the object config files.  Regular expression
diff --git a/tests/etc/2291.cfg b/tests/etc/2291.cfg
new file mode 100644
index 0000000..484942f
--- /dev/null
+++ b/tests/etc/2291.cfg
@@ -0,0 +1,64 @@
+#############################################################################################
+# ICINGA TEST CONFIG BY ISSUES
+# (c) 2009-2012 Icinga Development Team
+#
+# #2291
+# non existant macros are not replaced, and misleading to single dollar signs
+#############################################################################################
+
+
+define host{
+	name				2291linux-server
+	use				generic-host
+	check_period			24x7
+	check_interval			5
+	retry_interval			1
+	max_check_attempts		10
+        check_command           	2291macro_test1!20!50
+	notification_period		workhours
+	notification_interval		120
+	notification_options		d,u,r
+	contact_groups			testconfig-group-admin
+	register			0
+}
+
+define host{
+        use                     	2291linux-server
+        host_name               	2291localhost
+        display_name            	2291display_localhost
+        alias                   	2291localhost
+        address                 	127.0.0.1
+}
+
+define command{
+        command_name                    2291macro_test1
+        command_line                    $USER1$/check_procs -w $ARG1$ -c $ARG2$ -C $UNKNOWNHOSTMACRO1$
+}
+
+define command{
+        command_name                    2291macro_test2
+        command_line                    $USER1$/check_procs -w $ARG1$ -c $ARG2$ -C $UNKNOWSERVICENMACRO1$
+}
+
+define service {
+	name                           	2291default-servicecheck
+	register                       	0
+	service_description             2291Macro Test
+	check_command                   2291macro_test2!5!10
+	max_check_attempts              3
+	check_interval                  7
+	retry_interval                  3
+	active_checks_enabled           1
+	check_period                    24x7
+	obsess_over_service             0
+	check_freshness                 1
+	event_handler_enabled           0
+	notification_interval           10
+}
+
+define service {
+	use                             2291default-servicecheck
+	host_name			2291localhost
+	name                            2291Macro Test
+	service_description             2291Macro Test
+}





More information about the icinga-checkins mailing list