On Thu, Sep 05, 2024 at 15:49:37 +0200, Ján Tomko wrote:
> While the parsing is still done by 1K buffers, the results
> are no longer filtered during the parsing, but the whole JSON
> has to live in memory at once, which was also the case before
> the NSS plugin dropped its dependency on libvirt_util.
>
> Also, the new parser might be more forgiving of missing elements.
>
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
> tools/nss/libvirt_nss_leases.c | 370 +++++++++++----------------------
> 1 file changed, 119 insertions(+), 251 deletions(-)
[...]
> @@ -156,190 +128,120 @@ appendAddr(const char *name __attribute__((unused)),
> }
Subsequent hunks have the deletions trimmed to just show the new code.
>
>
> +/**
> + * findLeaseInJSON
> + *
> + * @jobj: the json object containing the leases
> + * @name: the requested hostname (optional if a MAC address is present)
> + * @macs: the array of MAC addresses we're matching (optional if we have a hostname)
> + * @nmacs: the size of the MAC array
> + * @af: the requested address family
> + * @now: current time (to eliminate expired leases)
> + * @addrs: the returned matching addresses
> + * @naddrs: size of the returned array
> + * @found: whether a match was found
> + *
> + * Returns 0 even if nothing was found
> + * -1 on error
> + */
> static int
> +findLeaseInJSON(json_object *jobj,
> + const char *name,
> + char **macs,
> + size_t nmacs,
> + int af,
> + time_t now,
> + leaseAddress **addrs,
> + size_t *naddrs,
> + bool *found)
> {
> size_t i;
> + int len;
>
> + if (!json_object_is_type(jobj, json_type_array)) {
> + ERROR("parsed JSON does not contain the leases array");
> + return -1;
> }
>
> + len = json_object_array_length(jobj);
> + for (i = 0; i < len; i++) {
> + json_object *lease = NULL;
> + json_object *expiry = NULL;
> + json_object *ipobj = NULL;
> + unsigned long long expiryTime;
> + const char *ipaddr;
> +
> + lease = json_object_array_get_idx(jobj, i);
> +
> + if (macs) {
> + const char *macAddr;
> + bool match = false;
> + json_object *val;
> + size_t j;
> +
> + val = json_object_object_get(lease, "mac-address");
> + if (!val)
> + continue;
> +
> + macAddr = json_object_get_string(val);
> + if (!macAddr)
> + continue;
> +
> + for (j = 0; j < nmacs; j++) {
> + if (!strcmp(macs[j], macAddr)) {
strcmp(..) != 0
> + match = true;
> + break;
> + }
> + }
> + if (!match)
> + continue;
> + } else {
> + const char *leaseName;
> + json_object *val;
> +
> + val = json_object_object_get(lease, "hostname");
> + if (!val)
> + continue;
> +
> + leaseName = json_object_get_string(val);
> + if (!leaseName)
> + continue;
> +
> + if (strcasecmp(leaseName, name) != 0)
> + continue;
> + }
>
> + expiry = json_object_object_get(lease, "expiry-time");
> + if (!expiry) {
> + ERROR("Missing expiry time for %s", name);
> + return -1;
> + }
>
> + expiryTime = json_object_get_uint64(expiry);
> + if (expiryTime > 0 && expiryTime < now) {
> + DEBUG("Skipping expired lease for %s", name);
> + continue;
> + }
>
> + ipobj = json_object_object_get(lease, "ip-address");
> + if (!ipobj) {
> + DEBUG("Missing IP address for %s", name);
> + continue;
> + }
> + ipaddr = json_object_get_string(ipobj);
>
> + DEBUG("Found record for %s", name);
> + *found = true;
>
> + if (appendAddr(name,
> + addrs, naddrs,
> + ipaddr,
> + expiryTime,
> + af) < 0)
> + return -1;
> }
>
> + return 0;
> }
>
>
> @@ -356,30 +258,10 @@ findLeases(const char *file,
> {
> int fd = -1;
> int ret = -1;
> + json_object *jobj = NULL;
> + json_tokener *tok = NULL;
> + enum json_tokener_error jerr;
> + int jsonflags = JSON_TOKENER_STRICT | JSON_TOKENER_VALIDATE_UTF8;
> char line[1024];
> ssize_t nreadTotal = 0;
> int rv;
> @@ -389,51 +271,37 @@ findLeases(const char *file,
> goto cleanup;
> }
>
> + tok = json_tokener_new();
> + json_tokener_set_flags(tok, jsonflags);
>
> + do {
> + rv = read(fd, line, sizeof(line) - 1);
-1 should not be needed here
> if (rv < 0)
> goto cleanup;
> if (rv == 0)
> break;
So if you get an incomplete JSON document of exactly sizeof(len) bytes,
this will go through the first loop adn thend break out ...
> nreadTotal += rv;
>
> + jobj = json_tokener_parse_ex(tok, line, rv);
> + jerr = json_tokener_get_error(tok);
> + } while (jerr == json_tokener_continue);
>
> + if (nreadTotal > 0 && jerr != json_tokener_success) {
... this condition will match as we read something and 'jerr' is still
'json_tokener_continue'
> + ERROR("Cannot parse %s: %s", file, json_tokener_error_desc(jerr));
So this error will read:
Cannot parse /path/to/file: continue
Thus you also need to specialcase also json_tokener_continue case to
state that the file is incomplete.
> goto cleanup;
> }
>
> + ret = findLeaseInJSON(jobj, name, macs, nmacs, af, now,
> + addrs, naddrs, found);
>
> cleanup:
> + json_object_put(jobj);
> + json_tokener_free(tok);
> if (ret != 0) {
> free(*addrs);
> *addrs = NULL;
> *naddrs = 0;
> }
> if (fd != -1)
> close(fd);
> return ret;
With the corner case fixed:
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
