[PATCH 0/4] Check that max_clients is not less than max_anonymous_clients
One check is there, but there ought to be more. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2033879 Martin Kletzander (4): Fix test case to actually test something tests: Check error message in virnetdaemontest rpc: Fix error message in virNetServerSetClientLimits rpc: Check client limits in more places src/rpc/virnetserver.c | 19 ++++++++++-- ...nput-data-client-auth-pending-failure.json | 4 +-- ...nput-data-invalid-max-clients-failure.json | 31 +++++++++++++++++++ .../output-data-anon-clients.err | 1 + ...utput-data-client-auth-pending-failure.err | 1 + ...utput-data-invalid-max-clients-failure.err | 1 + tests/virnetdaemontest.c | 20 +++++++----- 7 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-invalid-max-clients-failure.json create mode 100644 tests/virnetdaemondata/output-data-anon-clients.err create mode 100644 tests/virnetdaemondata/output-data-client-auth-pending-failure.err create mode 100644 tests/virnetdaemondata/output-data-invalid-max-clients-failure.err -- 2.39.0
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .../input-data-client-auth-pending-failure.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/virnetdaemondata/input-data-client-auth-pending-failure.json b/tests/virnetdaemondata/input-data-client-auth-pending-failure.json index 9c33d92e0824..da47fc7de63f 100644 --- a/tests/virnetdaemondata/input-data-client-auth-pending-failure.json +++ b/tests/virnetdaemondata/input-data-client-auth-pending-failure.json @@ -22,7 +22,7 @@ "isClient": false } ] - }, + } ], "clients": [ { @@ -38,7 +38,7 @@ "isClient": true }, "privateData": 1729 - }, + } ] } } -- 2.39.0
This way we actually check for the proper error, not any error like invalid JSON format. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .../output-data-anon-clients.err | 1 + ...utput-data-client-auth-pending-failure.err | 1 + tests/virnetdaemontest.c | 19 +++++++++++-------- 3 files changed, 13 insertions(+), 8 deletions(-) create mode 100644 tests/virnetdaemondata/output-data-anon-clients.err create mode 100644 tests/virnetdaemondata/output-data-client-auth-pending-failure.err diff --git a/tests/virnetdaemondata/output-data-anon-clients.err b/tests/virnetdaemondata/output-data-anon-clients.err new file mode 100644 index 000000000000..6d75757bd5e0 --- /dev/null +++ b/tests/virnetdaemondata/output-data-anon-clients.err @@ -0,0 +1 @@ +internal error: Server testServer1 was not created diff --git a/tests/virnetdaemondata/output-data-client-auth-pending-failure.err b/tests/virnetdaemondata/output-data-client-auth-pending-failure.err new file mode 100644 index 000000000000..d76079f0c94b --- /dev/null +++ b/tests/virnetdaemondata/output-data-client-auth-pending-failure.err @@ -0,0 +1 @@ +internal error: Invalid auth_pending and auth combination in JSON state document diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 2a8bc0ec9343..47d0923bdd93 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -303,8 +303,8 @@ static int testExecRestart(const void *opaque) infile = g_strdup_printf("%s/virnetdaemondata/input-data-%s.json", abs_srcdir, data->jsonfile); - outfile = g_strdup_printf("%s/virnetdaemondata/output-data-%s.json", - abs_srcdir, data->jsonfile); + outfile = g_strdup_printf("%s/virnetdaemondata/output-data-%s.%s", + abs_srcdir, data->jsonfile, data->pass ? "json" : "err"); if (virFileReadAll(infile, 8192, &injsonstr) < 0) goto cleanup; @@ -331,6 +331,9 @@ static int testExecRestart(const void *opaque) if (!(outjson = virNetDaemonPreExecRestart(dmn))) goto cleanup; + if (!data->pass) + goto cleanup; + if (!(outjsonstr = virJSONValueToString(outjson, true))) goto cleanup; @@ -340,15 +343,15 @@ static int testExecRestart(const void *opaque) ret = 0; cleanup: if (ret < 0) { - if (!data->pass) { - VIR_TEST_DEBUG("Got expected error: %s", - virGetLastErrorMessage()); + if (injson && !data->pass) { + ret = virTestCompareToFile(virGetLastErrorMessage(), outfile); + if (ret < 0) + VIR_TEST_DEBUG("Test failed with different error message"); virResetLastError(); - ret = 0; } } else if (!data->pass) { - VIR_TEST_DEBUG("Test should have failed"); - ret = -1; + VIR_TEST_DEBUG("Test should have failed"); + ret = -1; } virObjectUnref(dmn); VIR_FORCE_CLOSE(fdserver[0]); -- 2.39.0
That way it actually fits with what the condition checks for. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetserver.c | 6 ++-- ...nput-data-invalid-max-clients-failure.json | 31 +++++++++++++++++++ ...utput-data-invalid-max-clients-failure.err | 1 + tests/virnetdaemontest.c | 1 + 4 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-invalid-max-clients-failure.json create mode 100644 tests/virnetdaemondata/output-data-invalid-max-clients-failure.err diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 17ab61b88245..2ec4b9a6c947 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1114,9 +1114,9 @@ virNetServerSetClientLimits(virNetServer *srv, if (max < max_unauth) { virReportError(VIR_ERR_INVALID_ARG, "%s", - _("The overall maximum number of clients must be " - "greater than the maximum number of clients waiting " - "for authentication")); + _("The overall maximum number of clients waiting " + "for authentication must not be less than the overall " + "maximum number of clients")); return -1; } diff --git a/tests/virnetdaemondata/input-data-invalid-max-clients-failure.json b/tests/virnetdaemondata/input-data-invalid-max-clients-failure.json new file mode 100644 index 000000000000..9bd55929943b --- /dev/null +++ b/tests/virnetdaemondata/input-data-invalid-max-clients-failure.json @@ -0,0 +1,31 @@ +{ + "servers": { + "testServer0": { + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 5, + "max_anonymous_clients": 10, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "next_client_id": 5, + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + ] + } + } +} diff --git a/tests/virnetdaemondata/output-data-invalid-max-clients-failure.err b/tests/virnetdaemondata/output-data-invalid-max-clients-failure.err new file mode 100644 index 000000000000..00c2cee42c8c --- /dev/null +++ b/tests/virnetdaemondata/output-data-invalid-max-clients-failure.err @@ -0,0 +1 @@ +internal error: The overall maximum number of clients must not be less than the number of clients waiting for authentication diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 47d0923bdd93..110ec748f8b6 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -413,6 +413,7 @@ mymain(void) EXEC_RESTART_TEST_FAIL("anon-clients", 2); EXEC_RESTART_TEST("client-auth-pending", 1); EXEC_RESTART_TEST_FAIL("client-auth-pending-failure", 1); + EXEC_RESTART_TEST_FAIL("invalid-max-clients-failure", 1); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.39.0
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2033879 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetserver.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 2ec4b9a6c947..bf0fda04ee89 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -370,6 +370,13 @@ virNetServerNew(const char *name, g_autoptr(virNetServer) srv = NULL; g_autofree char *jobName = g_strdup_printf("rpc-%s", name); + if (max_clients < max_anonymous_clients) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("The overall maximum number of clients must not be less " + "than the number of clients waiting for authentication")); + return NULL; + } + if (virNetServerInitialize() < 0) return NULL; @@ -449,6 +456,12 @@ virNetServerNewPostExecRestart(virJSONValue *object, _("Malformed max_anonymous_clients data in JSON document")); return NULL; } + if (max_clients < max_anonymous_clients) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("The overall maximum number of clients must not be less " + "than the number of clients waiting for authentication")); + return NULL; + } } else { max_anonymous_clients = max_clients; } -- 2.39.0
On a Monday in 2023, Martin Kletzander wrote:
One check is there, but there ought to be more.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2033879
Martin Kletzander (4): Fix test case to actually test something tests: Check error message in virnetdaemontest rpc: Fix error message in virNetServerSetClientLimits rpc: Check client limits in more places
src/rpc/virnetserver.c | 19 ++++++++++-- ...nput-data-client-auth-pending-failure.json | 4 +-- ...nput-data-invalid-max-clients-failure.json | 31 +++++++++++++++++++ .../output-data-anon-clients.err | 1 + ...utput-data-client-auth-pending-failure.err | 1 + ...utput-data-invalid-max-clients-failure.err | 1 + tests/virnetdaemontest.c | 20 +++++++----- 7 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-invalid-max-clients-failure.json create mode 100644 tests/virnetdaemondata/output-data-anon-clients.err create mode 100644 tests/virnetdaemondata/output-data-client-auth-pending-failure.err create mode 100644 tests/virnetdaemondata/output-data-invalid-max-clients-failure.err
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Martin Kletzander