[PATCH 0/3] fail on non-option arguments, take 2

[Dominique Martinet]

This should fix the problem Christian reported and at any rate will
allow finer control.

The suricatta patch ended up more entengled than I had intended, see the
commit message there for details -- tl;dr is I think we can do better
but this is probably good enough for now.
If someone wants to do more I'll be glad to review/test.


Dominique Martinet (3):
cli: downloader: fail on non-option arguments to '-d'
cli: mongoose: fail on non-option arguments to '-w'
cli: suricatta: fail on non-option argument to '-u' for server_general

corelib/downloader.c | 5 +++++
mongoose/mongoose_interface.c | 6 ++++++
suricatta/server_general.c | 5 +++++
suricatta/server_hawkbit.c | 14 ++++++--------
4 files changed, 22 insertions(+), 8 deletions(-)

--
2.47.2

[Dominique Martinet]

This is the downloader part of commit 14bed359464b ("cli: fail if
non-option arguments passed to sub-arguments") without any modification.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---
corelib/downloader.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/corelib/downloader.c b/corelib/downloader.c
index 1348f12bfe38..81015ca25a57 100644
--- a/corelib/downloader.c
+++ b/corelib/downloader.c
@@ -254,6 +254,11 @@ int start_download_server(const char *fname, int argc, char *argv[])
return -EINVAL;
}
}
+ if (optind < argc) {
+ ERROR("Non-option or unrecognized argument(s) given to downloader (-d), see --help.");
+ download_print_help();
+ return -EINVAL;
+ }

/*
* if URL is passed, this is a one time step
--
2.47.2

[Dominique Martinet]

This is the downloader part of commit 14bed359464b ("cli: fail if
non-option arguments passed to sub-arguments") without any modification.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

mongoose/mongoose_interface.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
index 1fc4e6118285..9e09533a8a46 100644
--- a/mongoose/mongoose_interface.c
+++ b/mongoose/mongoose_interface.c
@@ -908,6 +908,12 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[])
}
}

+ if (optind < argc) {
+ ERROR("Non-option or unrecognized argument(s) given to webserver (-w), see --help.");
+ mongoose_print_help();
+ return -EINVAL;
+ }
+
s_http_server_opts.root_dir =
opts.root ? opts.root : MG_ROOT;
if (!opts.listing)
--
2.47.2

[Dominique Martinet]

The suricatta part of commit commit 14bed359464b ("cli: fail if
non-option arguments passed to sub-arguments") was not correct because
suricatta splits its parsing between start_suricatta() and each server's
server_start() functions, so the common code cannot error on leftover
non-option arguments (which actually are options' parameters)

Instead, add the check to server_general that knows what arguments to
expect (server_hawkbit already had the check, server_lua would need to
check in the user's lua code)

Note that unlike the downloader/mongoose added in previous commit, this
does not fail on unknown arguments (e.g. --foobar would be silently
ignored)
In practice, this already only works because 'S:' is declared in getopt
parameters, so we could added e/d as well and also check for invalid
arguments without too much effort if desired.
(If we did not have 'S:' in getopt arguments, we'd be left with -S'
parameter as positional argument, which I assume is why it was added in
the first place)

While here, harmonize the style of server_hawkbit's check:
- check optind < argc like the rest of the code for easier grepping
(the case compares optind >= argc with optind first so it also makes
sense from this point of view)
- use ERROR instead of fprintf like server_general

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

suricatta/server_general.c | 5 +++++
suricatta/server_hawkbit.c | 14 ++++++--------

2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/suricatta/server_general.c b/suricatta/server_general.c
index f106bce13437..f78ae6b609eb 100644
--- a/suricatta/server_general.c
+++ b/suricatta/server_general.c
@@ -685,6 +685,11 @@ static server_op_res_t server_start(const char *fname, int argc, char *argv[])
suricatta_print_help();
return SERVER_EINIT;
}
+ if (optind < argc) {
+ ERROR("Unused argument(s) given to suricatta (-u), see --help.");
+ suricatta_print_help();
+ return SERVER_EINIT;
+ }

channel_data_defaults.headers_to_send = &server_general.httpheaders_to_send;

diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
index 342670cd1ca4..519638c17eda 100644
--- a/suricatta/server_hawkbit.c
+++ b/suricatta/server_hawkbit.c
@@ -1826,9 +1826,7 @@ static server_op_res_t server_start(const char *fname, int argc, char *argv[])
case STATE_WAIT:
break;
default:
- fprintf(
- stderr,
- "Error: Invalid update status given.\n");
+ ERROR("Error: Invalid update status given.\n");
suricatta_print_help();
exit(EXIT_FAILURE);
}
@@ -1929,12 +1927,12 @@ static server_op_res_t server_start(const char *fname, int argc, char *argv[])
}

if (mandatory_argument_count != ALL_MANDATORY_SET) {
- fprintf(stderr, "Mandatory arguments missing!\n");
+ ERROR("Mandatory arguments missing!\n");
suricatta_print_help();
return SERVER_EINIT;
}
- if (argc > optind) {
- fprintf(stderr, "Unused arguments.\n");
+ if (optind < argc) {
+ ERROR("Unused argument(s) given to suricatta (-u), see --help.");
suricatta_print_help();
return SERVER_EINIT;
}
@@ -1951,8 +1949,8 @@ static server_op_res_t server_start(const char *fname, int argc, char *argv[])
* accept target and gateway token at the same time
*/
if (server_hawkbit.targettoken != NULL && server_hawkbit.gatewaytoken != NULL) {
- fprintf(stderr, "Error: both target and gateway tokens have been provided, "
- "but just one at a time is supported.\n");
+ ERROR("both target and gateway tokens have been provided, "
+ "but just one at a time is supported.\n");
exit(EXIT_FAILURE);
}
server_hawkbit_settoken("TargetToken", server_hawkbit.targettoken);
--
2.47.2

[Stefano Babic]

Hi Dominique,

Applied to -master, thanks !

Best regards,
Stefano Babic