[PATCH 1/3] Macros: Fix encoding of macros.

Max Sikstrom max.sikstrom at op5.com
Wed May 22 11:50:31 CEST 2013


From: Max Sikström <msikstrom at op5.com>

No macros should be escaped because of which macros is resolved, but for where
the macros is put.

URL-encoding macros should be done when building an URL, not if the URL is
included in the macro. Therefore, encoding options should only be brought
downwards, and only one level in the encoding.

An example:

notes: this is a string escaped with & and other chars for $HOSTADDRESS$
notes_url: http://wiki.example.org/?notes=$HOSTNOTES$&address=$HOSTADDRESS$
action_url: http://example.org/?notes_url=$HOSTNOTESURL$&notes=$HOSTNOTES$

The result should be something like:

notes:
address should be included, but not escaped. It should be readable by a person

notes_url:
both notes and address should be url-encoded, because they are a part of the
url, but shouldn't change the structure of the url (like including new
variables)

action_url:
should urlencode notes_url, which means parts is double-encoded, because the
notes_url-attribute should, when unpacked by the web server, contain the
notes_url, and the notes should be encoded, so the same applies to notes

Signed-off-by: Max Sikström <msikstrom at op5.com>
---
 common/macros.c |  101 ++++++++++++++++++++++--------------------------------
 1 files changed, 41 insertions(+), 60 deletions(-)

diff --git a/common/macros.c b/common/macros.c
index e76acef..b544dbb 100644
--- a/common/macros.c
+++ b/common/macros.c
@@ -38,7 +38,6 @@ char *macro_user[MAX_USER_MACROS]; /* $USERx$ macros */
 struct macro_key_code {
 	char *name; /* macro key name */
 	int code;  /* numeric macro code, usable in case statements */
-	int clean_options;
 	char *value;
 };
 
@@ -110,9 +109,7 @@ int process_macros_r(nagios_macros *mac, char *input_buffer, char **output_buffe
 	char *selected_macro = NULL;
 	char *original_macro = NULL;
 	int result = OK;
-	int clean_options = 0;
 	int free_macro = FALSE;
-	int macro_options = 0;
 
 	log_debug_info(DEBUGL_FUNCTIONS, 0, "process_macros_r()\n");
 
@@ -164,13 +161,10 @@ int process_macros_r(nagios_macros *mac, char *input_buffer, char **output_buffe
 		/* looks like we're in a macro, so process it... */
 		else {
 
-			/* reset clean options */
-			clean_options = 0;
-
 			/* grab the macro value */
 			free_macro = FALSE;
-			result = grab_macro_value_r(mac, temp_buffer, &selected_macro, &clean_options, &free_macro);
-			log_debug_info(DEBUGL_MACROS, 2, "  Processed '%s', Clean Options: %d, Free: %d\n", temp_buffer, clean_options, free_macro);
+			result = grab_macro_value_r(mac, temp_buffer, &selected_macro, NULL, &free_macro);
+			log_debug_info(DEBUGL_MACROS, 2, "  Processed '%s', Free: %d\n", temp_buffer, free_macro);
 
 			/* an error occurred - we couldn't parse the macro, so continue on */
 			if(result == ERROR) {
@@ -201,15 +195,10 @@ int process_macros_r(nagios_macros *mac, char *input_buffer, char **output_buffe
 
 			/* insert macro */
 			if(selected_macro != NULL) {
-				log_debug_info(DEBUGL_MACROS, 2, "  Processed '%s', Clean Options: %d, Free: %d\n", temp_buffer, clean_options, free_macro);
-
-				/* include any cleaning options passed back to us */
-				macro_options = (options | clean_options);
-
-				log_debug_info(DEBUGL_MACROS, 2, "  Cleaning options: global=%d, local=%d, effective=%d\n", options, clean_options, macro_options);
+				log_debug_info(DEBUGL_MACROS, 2, "  Processed '%s', Free: %d,  Cleaning options: %d\n", temp_buffer, free_macro, options);
 
 				/* URL encode the macro if requested - this allocates new memory */
-				if(macro_options & URL_ENCODE_MACRO_CHARS) {
+				if(options & URL_ENCODE_MACRO_CHARS) {
 					original_macro = selected_macro;
 					selected_macro = get_url_encoded_string(selected_macro);
 					if(free_macro == TRUE) {
@@ -219,11 +208,11 @@ int process_macros_r(nagios_macros *mac, char *input_buffer, char **output_buffe
 					}
 
 				/* some macros are cleaned... */
-				if(macro_options & STRIP_ILLEGAL_MACRO_CHARS || macro_options & ESCAPE_MACRO_CHARS) {
+				if((options & STRIP_ILLEGAL_MACRO_CHARS) || (options & ESCAPE_MACRO_CHARS)) {
 					char *cleaned_macro = NULL;
 
 					/* add the (cleaned) processed macro to the end of the already processed buffer */
-					if(selected_macro != NULL && (cleaned_macro = clean_macro_chars(selected_macro, macro_options)) != NULL) {
+					if(selected_macro != NULL && (cleaned_macro = clean_macro_chars(selected_macro, options)) != NULL) {
 						*output_buffer = (char *)realloc(*output_buffer, strlen(*output_buffer) + strlen(cleaned_macro) + 1);
 						strcat(*output_buffer, cleaned_macro);
 						if(*cleaned_macro)
@@ -413,7 +402,7 @@ int grab_macro_value_r(nagios_macros *mac, char *macro_buffer, char **output, in
 	/* clear the old macro value */
 	my_free(*output);
 
-	if(macro_buffer == NULL || clean_options == NULL || free_macro == NULL)
+	if(macro_buffer == NULL || free_macro == NULL)
 		return ERROR;
 
 
@@ -488,10 +477,6 @@ int grab_macro_value_r(nagios_macros *mac, char *macro_buffer, char **output, in
 
 	if ((mkey = find_macro_key(macro_name))) {
 		log_debug_info(DEBUGL_MACROS, 2, "  macros[%d] (%s) match.\n", mkey->code, macro_x_names[mkey->code]);
-		if (mkey->clean_options) {
-			*clean_options |= mkey->clean_options;
-			log_debug_info(DEBUGL_MACROS, 2, "  New clean options: %d\n", *clean_options);
-			}
 
 		/* get the macro value */
 		result = grab_macrox_value_r(mac, mkey->code, arg[0], arg[1], output, free_macro);
@@ -2389,12 +2374,6 @@ char *clean_macro_chars(char *macro, int options) {
 		ret[y++] = '\x0';
 		}
 
-#ifdef ON_HOLD_FOR_NOW
-	/* escape nasty character in macro */
-	if(options & ESCAPE_MACRO_CHARS) {
-		}
-#endif
-
 	return ret;
 	}
 
@@ -2402,6 +2381,32 @@ char *clean_macro_chars(char *macro, int options) {
 
 /* encodes a string in proper URL format */
 char *get_url_encoded_string(char *input) {
+	/* From RFC 3986:
+	segment       = *pchar
+
+	[...]
+
+	pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
+
+	query         = *( pchar / "/" / "?" )
+
+	fragment      = *( pchar / "/" / "?" )
+
+	pct-encoded   = "%" HEXDIG HEXDIG
+
+	unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
+	reserved      = gen-delims / sub-delims
+	gen-delims    = ":" / "/" / "?" / "#" / "[" / "]" / "@"
+	sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
+	                 / "*" / "+" / "," / ";" / "="
+
+	Encode everything but "unreserved", to be on safe side.
+
+	Another note:
+	nowhere in the RFC states that + is interpreted as space. Therefore, encode
+	space as %20 (as all other characters that should be escaped)
+	*/
+
 	register int x = 0;
 	register int y = 0;
 	char *encoded_url_string = NULL;
@@ -2420,17 +2425,17 @@ char *get_url_encoded_string(char *input) {
 	for(x = 0, y = 0; input[x] != (char)'\x0'; x++) {
 
 		/* alpha-numeric characters and a few other characters don't get encoded */
-		if(((char)input[x] >= '0' && (char)input[x] <= '9') || ((char)input[x] >= 'A' && (char)input[x] <= 'Z') || ((char)input[x] >= (char)'a' && (char)input[x] <= (char)'z') || (char)input[x] == (char)'.' || (char)input[x] == (char)'-' || (char)input[x] == (char)'_' || (char)input[x] == (char)':' || (char)input[x] == (char)'/' || (char)input[x] == (char)'?' || (char)input[x] == (char)'=' || (char)input[x] == (char)'&') {
+		if(		((char)input[x] >= '0' && (char)input[x] <= '9') ||
+				((char)input[x] >= 'A' && (char)input[x] <= 'Z') ||
+				((char)input[x] >= 'a' && (char)input[x] <= 'z') ||
+				(char)input[x] == '.' ||
+				(char)input[x] == '-' ||
+				(char)input[x] == '_' ||
+				(char)input[x] == '~') {
 			encoded_url_string[y] = input[x];
 			y++;
 			}
 
-		/* spaces are pluses */
-		else if((char)input[x] <= (char)' ') {
-			encoded_url_string[y] = '+';
-			y++;
-			}
-
 		/* anything else gets represented by its hex value */
 		else {
 			encoded_url_string[y] = '\x0';
@@ -2492,31 +2497,7 @@ int init_macros(void) {
 	for (x = 0; x < MACRO_X_COUNT; x++) {
 		macro_keys[x].code = x;
 		macro_keys[x].name = macro_x_names[x];
-		macro_keys[x].clean_options = 0;
-
-		switch (x) {
-			/* output, perfdata, comments and author names need cleaning */
-		case MACRO_HOSTOUTPUT: case MACRO_SERVICEOUTPUT:
-		case MACRO_HOSTPERFDATA: case MACRO_SERVICEPERFDATA:
-		case MACRO_HOSTACKAUTHOR: case MACRO_HOSTACKCOMMENT:
-		case MACRO_SERVICEACKAUTHOR: case MACRO_SERVICEACKCOMMENT:
-		case MACRO_LONGHOSTOUTPUT: case MACRO_LONGSERVICEOUTPUT:
-		case MACRO_HOSTGROUPNOTES: case MACRO_SERVICEGROUPNOTES:
-			macro_keys[x].clean_options = (STRIP_ILLEGAL_MACRO_CHARS | ESCAPE_MACRO_CHARS);
-			break;
-
-			/* url macros get url-encoded */
-		case MACRO_HOSTACTIONURL: case MACRO_HOSTNOTESURL:
-		case MACRO_SERVICEACTIONURL: case MACRO_SERVICENOTESURL:
-		case MACRO_HOSTGROUPNOTESURL: case MACRO_HOSTGROUPACTIONURL:
-		case MACRO_SERVICEGROUPNOTESURL: case MACRO_SERVICEGROUPACTIONURL:
-			macro_keys[x].clean_options = URL_ENCODE_MACRO_CHARS;
-			break;
-		default:
-			macro_keys[x].clean_options = 0;
-			break;
-			}
-		}
+	}
 
 	qsort(macro_keys, x, sizeof(struct macro_key_code), macro_key_cmp);
 	return OK;
-- 
1.7.1


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
_______________________________________________
Nagios-devel mailing list
Nagios-devel at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nagios-devel


More information about the Developers mailing list