9:59 a.m.
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
9:59 a.m.
New subject: [PATCH 1/4] Fix test case to actually test something
Signed-off-by: Martin Kletzander <mkletzan(a)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
9:59 a.m.
New subject: [PATCH 2/4] tests: Check error message in virnetdaemontest
This way we actually check for the proper error, not any error like invalid JSON
format.
Signed-off-by: Martin Kletzander <mkletzan(a)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
9:59 a.m.
New subject: [PATCH 3/4] rpc: Fix error message in virNetServerSetClientLimits
That way it actually fits with what the condition checks for.
Signed-off-by: Martin Kletzander <mkletzan(a)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
9:59 a.m.
New subject: [PATCH 4/4] rpc: Check client limits in more places
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2033879
Signed-off-by: Martin Kletzander <mkletzan(a)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
10:25 a.m.
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(a)redhat.com>
Jano
690
days inactive
690
days old
5 comments
2 participants
participants (2)
-
Ján Tomko -
Martin Kletzander