[PATCH] iptables: fix error reporting for missing/wrong arguments

[PATCH] iptables: fix error reporting for missing/wrong arguments

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

Hi,

I just noticed some problems in the iptables error reporting, the patch
attached should fix them. If nobody finds any problem on it, I'll push
it to git.

Some work to improve iptables error reporting would be nice, I'll see if
I can have a look into this issue. BTW, it would be nice to have some
kind of tool to automate the testing of command line interface. Thus, we
can ensure that the tool output remains consistent. I started some tool
for the conntrack-tools but it is still very simple.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 2449 bytes --]

[PATCH] iptables: fix error reporting for missing/wrong arguments

This patch fixes a confusing error reporting when some arguments are
missing or the option passed is unknown.

According to man getopt_long:
"If getopt() finds an option character in argv that was not
 included in optstring, or if it detects a missing option 
 argument, it returns '?' and sets the external variable
 optopt to the actual option character."

Without the patch:

% iptables -I INPUT -m state --whatever
iptables v1.4.2-rc1: Unknown arg `(null)

% iptables -I INPUT -m state --state
iptables v1.4.2-rc1: Unknown arg `(null)

With the patch:

% iptables -I INPUT -m state --whatever
iptables v1.4.2-rc1: unknown option `--whatever'
Try `iptables -h' or 'iptables --help' for more information.

% iptables -I INPUT -m state --state
iptables v1.4.2-rc1: option `--state' requires an argument
Try `iptables -h' or 'iptables --help' for more information.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/ip6tables.c b/ip6tables.c
index 12298ca..9ce1074 100644
--- a/ip6tables.c
+++ b/ip6tables.c
@@ -1888,9 +1888,26 @@ int do_command6(int argc, char *argv[], char **table, ip6tc_handle_t *handle)
 					continue;
 				}
 
-				if (!m)
+				if (!m) {
+					if (c == '?') {
+						if (optopt) {
+							exit_error(
+							   PARAMETER_PROBLEM,
+							   "option `%s' "
+							   "requires an "
+							   "argument",
+							   argv[optind-1]);
+						} else {
+							exit_error(
+							   PARAMETER_PROBLEM,
+							   "unknown option "
+							   "`%s'",
+							   argv[optind-1]);
+						}
+					}
 					exit_error(PARAMETER_PROBLEM,
 						   "Unknown arg `%s'", optarg);
+				}
 			}
 		}
 		invert = FALSE;
diff --git a/iptables.c b/iptables.c
index b927a11..d2b9081 100644
--- a/iptables.c
+++ b/iptables.c
@@ -1909,9 +1909,26 @@ int do_command(int argc, char *argv[], char **table, iptc_handle_t *handle)
 					optind--;
 					continue;
 				}
-				if (!m)
+				if (!m) {
+					if (c == '?') {
+						if (optopt) {
+							exit_error(
+							   PARAMETER_PROBLEM,
+							   "option `%s' "
+							   "requires an "
+							   "argument",
+							   argv[optind-1]);
+						} else {
+							exit_error(
+							   PARAMETER_PROBLEM,
+							   "unknown option "
+							   "`%s'",
+							   argv[optind-1]);
+						}
+					}
 					exit_error(PARAMETER_PROBLEM,
 						   "Unknown arg `%s'", optarg);
+				}
 			}
 		}
 		invert = FALSE;

Re: [PATCH] iptables: fix error reporting for missing/wrong arguments

[Jan Engelhardt]

On Sunday 2008-10-12 15:39, Pablo Neira Ayuso wrote:
>
>Some work to improve iptables error reporting would be nice, I'll see if
>I can have a look into this issue.

Improve - in what way?

Re: [PATCH] iptables: fix error reporting for missing/wrong arguments

[Pablo Neira Ayuso]

Jan Engelhardt wrote:
> On Sunday 2008-10-12 15:39, Pablo Neira Ayuso wrote:
>> Some work to improve iptables error reporting would be nice, I'll see if
>> I can have a look into this issue.
> 
> Improve - in what way?

There are a couple of nitpicking issues that I have observed during some
training sessions, dummy users have the ability to trigger very
confusing error messages from iptables, just one example:

% iptables -I INPUT -m state --state NEW, ESTABLISHED
                                         ^
                                     mind the small space

triggers:

iptables v1.4.2-rc1: Bad state `'
Try `iptables -h' or 'iptables --help' for more information.

Another example: if iptables returns EINVAL, it would be nice to add a
string to say "Check your kernel log messages". Even if we don't have
better error handling, at least we can be more informative.

So, when I said *improve* I meant about hunting this sort of issues.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers