Blob Blame History Raw
From fc87d26b0343a5fbe661acc967f7a7c316531ca5 Mon Sep 17 00:00:00 2001
From: Phil Sutter <psutter@redhat.com>
Date: Wed, 3 Apr 2019 20:16:49 +0200
Subject: [PATCH] xshared: Consolidate argv construction routines

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1668475
Upstream Status: iptables commit a2ed880a19d08
Conflicts:
* Context change due to missing commit 2963a8df2175b
  ("iptables: Remove explicit static variables initalization.").
* Context change due to missing commit 1cc09188079a6
  ("xshared: Consolidate parse_counters()").
* Context change due to previously backported commit 8da04ffdca193
  ("Share print_ipv{4,6}_addr() from xtables").
* Dropped changes to non-existing file iptables/xtables-restore.c.

commit a2ed880a19d0861342b3515721804b18d698bf44
Author: Phil Sutter <phil@nwl.cc>
Date:   Thu Aug 2 17:05:17 2018 +0200

    xshared: Consolidate argv construction routines

    Implementations were equal in {ip,ip6,x}tables-restore.c. The one in
    iptables-xml.c differed slightly. For now, collect all features
    together. Maybe it would make sense to migrate iptables-xml.c to using
    add_param_to_argv() at some point and therefore extend the latter to
    store whether a given parameter was quoted or not.

    While being at it, a few improvements were done:

    * free_argv() now also resets 'newargc' variable, so users don't have to
      do that anymore.
    * Indenting level in add_param_to_argv() was reduced a bit.
    * That long error message is put into a single line to aid in grepping
      for it.
    * Explicit call to exit() after xtables_error() is removed since the
      latter does not return anyway.

    Signed-off-by: Phil Sutter <phil@nwl.cc>
    Signed-off-by: Florian Westphal <fw@strlen.de>

Signed-off-by: Phil Sutter <psutter@redhat.com>
---
 iptables/ip6tables-restore.c | 107 ++----------------------------
 iptables/iptables-restore.c  | 107 ++----------------------------
 iptables/iptables-xml.c      |  63 ------------------
 iptables/xshared.c           | 123 +++++++++++++++++++++++++++++++++++
 iptables/xshared.h           |  13 ++++
 5 files changed, 150 insertions(+), 263 deletions(-)

diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c
index 611430d930eda..1f8cb43286f03 100644
--- a/iptables/ip6tables-restore.c
+++ b/iptables/ip6tables-restore.c
@@ -91,96 +91,6 @@ static int parse_counters(char *string, struct xt_counters *ctr)
 	return ret == 2;
 }
 
-/* global new argv and argc */
-static char *newargv[255];
-static int newargc;
-
-/* function adding one argument to newargv, updating newargc
- * returns true if argument added, false otherwise */
-static int add_argv(char *what) {
-	DEBUGP("add_argv: %s\n", what);
-	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
-		newargv[newargc] = strdup(what);
-		newargv[++newargc] = NULL;
-		return 1;
-	} else {
-		xtables_error(PARAMETER_PROBLEM,
-			"Parser cannot handle more arguments\n");
-		return 0;
-	}
-}
-
-static void free_argv(void) {
-	int i;
-
-	for (i = 0; i < newargc; i++)
-		free(newargv[i]);
-}
-
-static void add_param_to_argv(char *parsestart)
-{
-	int quote_open = 0, escaped = 0, param_len = 0;
-	char param_buffer[1024], *curchar;
-
-	/* After fighting with strtok enough, here's now
-	 * a 'real' parser. According to Rusty I'm now no
-	 * longer a real hacker, but I can live with that */
-
-	for (curchar = parsestart; *curchar; curchar++) {
-		if (quote_open) {
-			if (escaped) {
-				param_buffer[param_len++] = *curchar;
-				escaped = 0;
-				continue;
-			} else if (*curchar == '\\') {
-				escaped = 1;
-				continue;
-			} else if (*curchar == '"') {
-				quote_open = 0;
-				*curchar = ' ';
-			} else {
-				param_buffer[param_len++] = *curchar;
-				continue;
-			}
-		} else {
-			if (*curchar == '"') {
-				quote_open = 1;
-				continue;
-			}
-		}
-
-		if (*curchar == ' '
-		    || *curchar == '\t'
-		    || * curchar == '\n') {
-			if (!param_len) {
-				/* two spaces? */
-				continue;
-			}
-
-			param_buffer[param_len] = '\0';
-
-			/* check if table name specified */
-			if (!strncmp(param_buffer, "-t", 2)
-                            || !strncmp(param_buffer, "--table", 8)) {
-				xtables_error(PARAMETER_PROBLEM,
-				"The -t option (seen in line %u) cannot be "
-				"used in ip6tables-restore.\n", line);
-				exit(1);
-			}
-
-			add_argv(param_buffer);
-			param_len = 0;
-		} else {
-			/* regular character, copy to buffer */
-			param_buffer[param_len++] = *curchar;
-
-			if (param_len >= sizeof(param_buffer))
-				xtables_error(PARAMETER_PROBLEM,
-				   "Parameter too long!");
-		}
-	}
-}
-
 int ip6tables_restore_main(int argc, char *argv[])
 {
 	struct xtc_handle *handle = NULL;
@@ -425,9 +335,6 @@ int ip6tables_restore_main(int argc, char *argv[])
 			char *bcnt = NULL;
 			char *parsestart;
 
-			/* reset the newargv */
-			newargc = 0;
-
 			if (buffer[0] == '[') {
 				/* we have counters in our input */
 				char *ptr = strchr(buffer, ']');
@@ -456,17 +363,17 @@ int ip6tables_restore_main(int argc, char *argv[])
 				parsestart = buffer;
 			}
 
-			add_argv(argv[0]);
-			add_argv("-t");
-			add_argv(curtable);
+			add_argv(argv[0], 0);
+			add_argv("-t", 0);
+			add_argv(curtable, 0);
 
 			if (counters && pcnt && bcnt) {
-				add_argv("--set-counters");
-				add_argv((char *) pcnt);
-				add_argv((char *) bcnt);
+				add_argv("--set-counters", 0);
+				add_argv((char *) pcnt, 0);
+				add_argv((char *) bcnt, 0);
 			}
 
-			add_param_to_argv(parsestart);
+			add_param_to_argv(parsestart, line);
 
 			DEBUGP("calling do_command6(%u, argv, &%s, handle):\n",
 				newargc, curtable);
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index b0da96d45d297..615e38a6625e0 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -89,96 +89,6 @@ static int parse_counters(char *string, struct xt_counters *ctr)
 	return ret == 2;
 }
 
-/* global new argv and argc */
-static char *newargv[255];
-static int newargc;
-
-/* function adding one argument to newargv, updating newargc 
- * returns true if argument added, false otherwise */
-static int add_argv(char *what) {
-	DEBUGP("add_argv: %s\n", what);
-	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
-		newargv[newargc] = strdup(what);
-		newargv[++newargc] = NULL;
-		return 1;
-	} else {
-		xtables_error(PARAMETER_PROBLEM,
-			"Parser cannot handle more arguments\n");
-		return 0;
-	}
-}
-
-static void free_argv(void) {
-	int i;
-
-	for (i = 0; i < newargc; i++)
-		free(newargv[i]);
-}
-
-static void add_param_to_argv(char *parsestart)
-{
-	int quote_open = 0, escaped = 0, param_len = 0;
-	char param_buffer[1024], *curchar;
-
-	/* After fighting with strtok enough, here's now
-	 * a 'real' parser. According to Rusty I'm now no
-	 * longer a real hacker, but I can live with that */
-
-	for (curchar = parsestart; *curchar; curchar++) {
-		if (quote_open) {
-			if (escaped) {
-				param_buffer[param_len++] = *curchar;
-				escaped = 0;
-				continue;
-			} else if (*curchar == '\\') {
-				escaped = 1;
-				continue;
-			} else if (*curchar == '"') {
-				quote_open = 0;
-				*curchar = ' ';
-			} else {
-				param_buffer[param_len++] = *curchar;
-				continue;
-			}
-		} else {
-			if (*curchar == '"') {
-				quote_open = 1;
-				continue;
-			}
-		}
-
-		if (*curchar == ' '
-		    || *curchar == '\t'
-		    || * curchar == '\n') {
-			if (!param_len) {
-				/* two spaces? */
-				continue;
-			}
-
-			param_buffer[param_len] = '\0';
-
-			/* check if table name specified */
-			if (!strncmp(param_buffer, "-t", 2)
-			    || !strncmp(param_buffer, "--table", 8)) {
-				xtables_error(PARAMETER_PROBLEM,
-				"The -t option (seen in line %u) cannot be "
-				"used in iptables-restore.\n", line);
-				exit(1);
-			}
-
-			add_argv(param_buffer);
-			param_len = 0;
-		} else {
-			/* regular character, copy to buffer */
-			param_buffer[param_len++] = *curchar;
-
-			if (param_len >= sizeof(param_buffer))
-				xtables_error(PARAMETER_PROBLEM,
-				   "Parameter too long!");
-		}
-	}
-}
-
 int
 iptables_restore_main(int argc, char *argv[])
 {
@@ -424,9 +334,6 @@ iptables_restore_main(int argc, char *argv[])
 			char *bcnt = NULL;
 			char *parsestart;
 
-			/* reset the newargv */
-			newargc = 0;
-
 			if (buffer[0] == '[') {
 				/* we have counters in our input */
 				char *ptr = strchr(buffer, ']');
@@ -455,17 +362,17 @@ iptables_restore_main(int argc, char *argv[])
 				parsestart = buffer;
 			}
 
-			add_argv(argv[0]);
-			add_argv("-t");
-			add_argv(curtable);
+			add_argv(argv[0], 0);
+			add_argv("-t", 0);
+			add_argv(curtable, 0);
 
 			if (counters && pcnt && bcnt) {
-				add_argv("--set-counters");
-				add_argv((char *) pcnt);
-				add_argv((char *) bcnt);
+				add_argv("--set-counters", 0);
+				add_argv((char *) pcnt, 0);
+				add_argv((char *) bcnt, 0);
 			}
 
-			add_param_to_argv(parsestart);
+			add_param_to_argv(parsestart, line);
 
 			DEBUGP("calling do_command4(%u, argv, &%s, handle):\n",
 				newargc, curtable);
diff --git a/iptables/iptables-xml.c b/iptables/iptables-xml.c
index c523a132b2240..49f8ea2826181 100644
--- a/iptables/iptables-xml.c
+++ b/iptables/iptables-xml.c
@@ -66,16 +66,6 @@ parse_counters(char *string, struct xt_counters *ctr)
 		return (0 == 2);
 }
 
-/* global new argv and argc */
-static char *newargv[255];
-static unsigned int newargc = 0;
-
-static char *oldargv[255];
-static unsigned int oldargc = 0;
-
-/* arg meta data, were they quoted, frinstance */
-static int newargvattr[255];
-
 #define XT_CHAIN_MAXNAMELEN XT_TABLE_MAXNAMELEN
 static char closeActionTag[XT_TABLE_MAXNAMELEN + 1];
 static char closeRuleTag[XT_TABLE_MAXNAMELEN + 1];
@@ -93,56 +83,6 @@ struct chain {
 static struct chain chains[maxChains];
 static int nextChain = 0;
 
-/* funCtion adding one argument to newargv, updating newargc 
- * returns true if argument added, false otherwise */
-static int
-add_argv(char *what, int quoted)
-{
-	DEBUGP("add_argv: %d %s\n", newargc, what);
-	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
-		newargv[newargc] = strdup(what);
-		newargvattr[newargc] = quoted;
-		newargc++;
-		return 1;
-	} else
-		return 0;
-}
-
-static void
-free_argv(void)
-{
-	unsigned int i;
-
-	for (i = 0; i < newargc; i++) {
-		free(newargv[i]);
-		newargv[i] = NULL;
-	}
-	newargc = 0;
-
-	for (i = 0; i < oldargc; i++) {
-		free(oldargv[i]);
-		oldargv[i] = NULL;
-	}
-	oldargc = 0;
-}
-
-/* save parsed rule for comparison with next rule 
-   to perform action agregation on duplicate conditions */
-static void
-save_argv(void)
-{
-	unsigned int i;
-
-	for (i = 0; i < oldargc; i++)
-		free(oldargv[i]);
-	oldargc = newargc;
-	newargc = 0;
-	for (i = 0; i < oldargc; i++) {
-		oldargv[i] = newargv[i];
-		newargv[i] = NULL;
-	}
-}
-
 /* like puts but with xml encoding */
 static void
 xmlEncode(char *text)
@@ -736,9 +676,6 @@ iptables_xml_main(int argc, char *argv[])
 			int quote_open, quoted;
 			char param_buffer[1024];
 
-			/* reset the newargv */
-			newargc = 0;
-
 			if (buffer[0] == '[') {
 				/* we have counters in our input */
 				char *ptr = strchr(buffer, ']');
diff --git a/iptables/xshared.c b/iptables/xshared.c
index 742502154aa55..84dbea562576e 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -406,3 +406,126 @@ void print_ipv6_addresses(const struct ip6t_entry *fw6, unsigned int format)
 	       ipv6_addr_to_string(&fw6->ipv6.dst,
 				   &fw6->ipv6.dmsk, format));
 }
+
+/* global new argv and argc */
+char *newargv[255];
+int newargc = 0;
+
+/* saved newargv and newargc from save_argv() */
+char *oldargv[255];
+int oldargc = 0;
+
+/* arg meta data, were they quoted, frinstance */
+int newargvattr[255];
+
+/* function adding one argument to newargv, updating newargc
+ * returns true if argument added, false otherwise */
+int add_argv(const char *what, int quoted)
+{
+	DEBUGP("add_argv: %s\n", what);
+	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
+		newargv[newargc] = strdup(what);
+		newargvattr[newargc] = quoted;
+		newargv[++newargc] = NULL;
+		return 1;
+	} else {
+		xtables_error(PARAMETER_PROBLEM,
+			      "Parser cannot handle more arguments\n");
+	}
+}
+
+void free_argv(void)
+{
+	while (newargc)
+		free(newargv[--newargc]);
+	while (oldargc)
+		free(oldargv[--oldargc]);
+}
+
+/* Save parsed rule for comparison with next rule to perform action aggregation
+ * on duplicate conditions.
+ */
+void save_argv(void)
+{
+	unsigned int i;
+
+	while (oldargc)
+		free(oldargv[--oldargc]);
+
+	oldargc = newargc;
+	newargc = 0;
+	for (i = 0; i < oldargc; i++) {
+		oldargv[i] = newargv[i];
+	}
+}
+
+void add_param_to_argv(char *parsestart, int line)
+{
+	int quote_open = 0, escaped = 0, param_len = 0;
+	char param_buffer[1024], *curchar;
+
+	/* After fighting with strtok enough, here's now
+	 * a 'real' parser. According to Rusty I'm now no
+	 * longer a real hacker, but I can live with that */
+
+	for (curchar = parsestart; *curchar; curchar++) {
+		if (quote_open) {
+			if (escaped) {
+				param_buffer[param_len++] = *curchar;
+				escaped = 0;
+				continue;
+			} else if (*curchar == '\\') {
+				escaped = 1;
+				continue;
+			} else if (*curchar == '"') {
+				quote_open = 0;
+				*curchar = '"';
+			} else {
+				param_buffer[param_len++] = *curchar;
+				continue;
+			}
+		} else {
+			if (*curchar == '"') {
+				quote_open = 1;
+				continue;
+			}
+		}
+
+		switch (*curchar) {
+		case '"':
+			break;
+		case ' ':
+		case '\t':
+		case '\n':
+			if (!param_len) {
+				/* two spaces? */
+				continue;
+			}
+			break;
+		default:
+			/* regular character, copy to buffer */
+			param_buffer[param_len++] = *curchar;
+
+			if (param_len >= sizeof(param_buffer))
+				xtables_error(PARAMETER_PROBLEM,
+					      "Parameter too long!");
+			continue;
+		}
+
+		param_buffer[param_len] = '\0';
+
+		/* check if table name specified */
+		if ((param_buffer[0] == '-' &&
+		     param_buffer[1] != '-' &&
+		     strchr(param_buffer, 't')) ||
+		    (!strncmp(param_buffer, "--t", 3) &&
+		     !strncmp(param_buffer, "--table", strlen(param_buffer)))) {
+			xtables_error(PARAMETER_PROBLEM,
+				      "The -t option (seen in line %u) cannot be used in %s.\n",
+				      line, xt_params->program_name);
+		}
+
+		add_argv(param_buffer, 0);
+		param_len = 0;
+	}
+}
diff --git a/iptables/xshared.h b/iptables/xshared.h
index bfdb10b2701e5..4f567db9f410b 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -119,6 +119,19 @@ bool xs_has_arg(int argc, char *argv[]);
 
 extern const struct xtables_afinfo *afinfo;
 
+extern char *newargv[];
+extern int newargc;
+
+extern char *oldargv[];
+extern int oldargc;
+
+extern int newargvattr[];
+
+int add_argv(const char *what, int quoted);
+void free_argv(void);
+void save_argv(void);
+void add_param_to_argv(char *parsestart, int line);
+
 void print_ipv4_addresses(const struct ipt_entry *fw, unsigned int format);
 void print_ipv6_addresses(const struct ip6t_entry *fw6, unsigned int format);
 
-- 
2.21.0