[PATCH] ifupdown: make parser for /etc/network/interfaces more robust

[Peter Marschall <peter adpm de>]

Hi,

the previous implementation of the parser for /etc/network/interfaces had
quite a few drawbacks:
- it expected the lines to be terminated with "\n", even the last line
- it ignored line wraps with "\\" followed by "\n"
- it expected over-long lines to be shorter than 510 characters
- it ignored line wraps on over-long lines
- it treated spaces and tabs differently
- it did not make sure to really tokenize on word boundaries
- it treated the equivalent stanzas "auto" and "allow-auto" differently
- it ignored the fact that the "allow-*" stanzas can take multiple arguments
  that need to be separated to be recognized NetworkManager's processing later
- it allowed "non-block" stanzas to appear before a block

The attached patch is a rewrite of the parser to fix the issues mentioned:
- it accepts the last line even if it is not terminated by "\n"
- it skips over-long lines, emits a warning and even takes int account
  that over-long lines may be wrapped to next lines
- it un-wraps wrapped lines
- it uses spaces and tabs equivalently to tokenize the input
- it treats "allow-auto" as a synonym to "auto"
- it splits multi-argument "auto"/"allow-*" stanza into multiple
  single-argument stanzas of the same type
- it warns on data stanzas before the first block stanza

Signed-off-by: Peter Marschall <peter adpm de>

Please consider integrating this patch into the next release of NetworkManager

Best
Peter

-- 
Peter Marschall
peter adpm de

From 464c030738ffca9cf85101d4b34e01e996f368e6 Mon Sep 17 00:00:00 2001
From: Peter Marschall <peter adpm de>
Date: Sun, 8 Aug 2010 14:41:54 +0200
Subject: [PATCH] ifupdown: make parser for /etc/network/interfaces more robust

The previous implementation of the parser for /etc/network/interfaces had
quite a few drawbacks:
- it expected the lines to be terminated with "\n", even the last line
- it ignored line wraps with "\\" followed by "\n"
- it expected over-long lines to be shorter than 510 characters
- it ignored line wraps on over-long lines
- it treated spaces and tabs differently
- it did not make sure to really tokenize on word boundaries
- it treated the equivalent stanzas "auto" and "allow-auto" differently
- it ignored the fact that the "allow-*" stanzas can take multiple arguments
  that need to be separated to be recognized NetworkManager's processing later
- it allowed "non-block" stanzas to appear before a block

This patch is a rewrite of the parser to fix the issues mentioned:
- it accepts the last line even if it is not terminated by "\n"
- it skips over-long lines, emits a warning and even takes into account
  that over-long lines may be wrapped to next lines
- it un-wraps wrapped lines
- it uses spaces and tabs equivalently to tokenize the input
- it treats "allow-auto" as a synonym to "auto"
- it splits multi-argument "auto"/"allow-*" into multiple
  single-argument stanzas of the same type
- it warns on data stanzas before the first block stanza

Signed-off-by: Peter Marschall <peter adpm de>
---
 .../plugins/ifupdown/interface_parser.c            |  161 +++++++++++++-------
 1 files changed, 107 insertions(+), 54 deletions(-)

diff --git a/system-settings/plugins/ifupdown/interface_parser.c b/system-settings/plugins/ifupdown/interface_parser.c
index f28cf7c..b55e31c 100644
--- a/system-settings/plugins/ifupdown/interface_parser.c
+++ b/system-settings/plugins/ifupdown/interface_parser.c
@@ -73,15 +73,30 @@ void add_data(const char *key,const char *data)
 	//printf("added data '%s' with key '%s'\n",data,key);
 }
 
-#define SPACE_OR_TAB(string,ret) {ret = strchr(string,' ');ret=(ret == NULL?strchr(string,'\t'):ret);}
+// join values in src with spaces into dst;  dst needs to be large enough
+static char *join_values_with_spaces(char *dst, char **src)
+{
+	if (dst != NULL) {
+		*dst = '\0';
+		if (src != NULL && *src != NULL) {
+			strcat(dst, *src);
+
+			for (src++; *src != NULL; src++) {
+				strcat(dst, " ");
+				strcat(dst, *src);
+			}
+		}
+	}
+	return(dst);
+}
 
 void ifparser_init(void)
 {
 	FILE *inp = fopen(ENI_INTERFACES_FILE, "r");
-	int ret = 0;
-	char *line;
-	char *space;
-	char rline[255];
+	char line[255];
+	int skip_to_block = 1;
+	int skip_long_line = 0;
+	int offs = 0;
 
 	if (inp == NULL)
 	{
@@ -89,69 +104,107 @@ void ifparser_init(void)
 		return;
 	}
 	first = last = NULL;
-	while(1)
+	while (!feof(inp))
 	{
-		line = space = NULL;
-		ret = fscanf(inp,"%255[^\n]\n",rline);
-		if (ret == EOF)
+		char *token[128];	// 255 chars can only be split into 127 tokens
+		char value[255];	// large enough to join previously split tokens
+		char *safeptr;
+		int toknum;
+		int len = 0;
+
+		char *ptr = fgets(line+offs, 255-offs, inp);
+		if (ptr == NULL)
 			break;
-		// If the line did not match, skip it
-		if (ret == 0) {
-			char *ignored;
 
-			ignored = fgets(rline, 255, inp);
+		len = strlen(line);
+		// skip over-long lines
+		if (!feof(inp) && len > 0 &&  line[len-1] != '\n') {
+			if (!skip_long_line)
+				nm_warning ("Error: Skipping over-long-line '%s...'\n", line);
+			skip_long_line = 1;
 			continue;
 		}
 
-		line = rline;
-		while(line[0] == ' ')
-			line++;
-		if (line[0]=='#' || line[0]=='\0')
+		// trailing '\n' found: remove it & reset offset to 0
+		if (len > 0 && line[len-1] == '\n') {
+			line[--len] = '\0';
+			offs = 0;
+		}
+
+		// if we're in long_line_skip mode, terminate it for real next line
+		if (skip_long_line) {
+			if (len == 0 || line[len-1] != '\\')
+				skip_long_line = 0;
 			continue;
+		}
 
-		SPACE_OR_TAB(line,space)
-			if (space == NULL)
-			{
-				nm_warning ("Error: Can't parse interface line '%s'\n",line);
-				continue;
-			}
-		space[0] = '\0';
+		// unwrap wrapped lines
+		if (len > 0 && line[len-1] == '\\') {
+			offs = len - 1;
+			continue;
+		}
+
+		//printf(">>%s<<\n", line);
+
+#define SPACES	" \t"
+		// tokenize input;
+		for (toknum = 0, token[toknum] = strtok_r(line, SPACES, &safeptr);
+		     token[toknum] != NULL;
+		     toknum++, token[toknum] = strtok_r(NULL, SPACES, &safeptr))
+			;
+
+		// ignore comments and empty lines
+		if (toknum == 0 || *token[0]=='#')
+			continue;
+
+		if (toknum < 2) {
+			nm_warning ("Error: Can't parse interface line '%s'\n",
+					join_values_with_spaces(value, token));
+			skip_to_block = 1;
+			continue;
+		}
 
 		// There are four different stanzas:
 		// iface, mapping, auto and allow-*. Create a block for each of them.
-		if (strcmp(line,"iface")==0)
-		{
-			char *space2 = strchr(space+1,' ');
-			if (space2 == NULL)
-			{
-				nm_warning ("Error: Can't parse iface line '%s'\n",space+1);
+
+		// iface stanza takes at least 3 parameters
+		if (strcmp(token[0], "iface") == 0) {
+			if (toknum < 4) {
+				nm_warning ("Error: Can't parse iface line '%s'\n",
+						join_values_with_spaces(value, token));
 				continue;
 			}
-			space2[0]='\0';
-			add_block(line,space+1);
-
-			if (space2[1]!='\0')
-			{
-				space = strchr(space2+1,' ');
-				if (space == NULL)
-				{
-					nm_warning ("Error: Can't parse data '%s'\n",space2+1);
-					continue;
-				}
-				space[0] = '\0';
-				add_data(space2+1,space+1);
-			}
+			add_block(token[0], token[1]);
+			skip_to_block = 0;
+			add_data(token[2], join_values_with_spaces(value, token + 3));
+		}
+		// auto and allow-auto stanzas are equivalent,
+		// both can take multiple interfaces as parameters: add one block for each
+		else if (strcmp(token[0], "auto") == 0 ||
+			 strcmp(token[0], "allow-auto") == 0) {
+			int i;
+			for (i = 1; i < toknum; i++)
+				add_block("auto", token[i]);
+			skip_to_block = 0;
+		}
+		else if (strcmp(token[0], "mapping") == 0) {
+			add_block(token[0], join_values_with_spaces(value, token + 1));
+			skip_to_block = 0;
+		}
+		// allow-* can take multiple interfaces as parameters: add one block for each
+		else if (strncmp(token[0],"allow-",6) == 0) {
+			int i;
+			for (i = 1; i < toknum; i++)
+				add_block(token[0], token[i]);
+			skip_to_block = 0;
+		}
+		else {
+			if (skip_to_block)
+				nm_warning ("Error: ignoring out-of-block data '%s'\n",
+						join_values_with_spaces(value, token));
+			else
+				add_data(token[0], join_values_with_spaces(value, token + 1));
 		}
-		else if (strcmp(line,"auto")==0)
-			add_block(line,space+1);
-		else if (strcmp(line,"mapping")==0)
-			add_block(line,space+1);
-		else if (strncmp(line,"allow-",6)==0)
-			add_block(line,space+1);
-		else
-			add_data(line,space+1);
-
-		//printf("line: '%s' ret=%d\n",rline,ret);
 	}
 	fclose(inp);
 }
-- 
1.7.1