diff --git a/NEWS.adoc b/NEWS.adoc index d3bd184fa1..d8ffdd7b68 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -137,6 +137,14 @@ https://github.com/networkupstools/nut/milestone/11 * `upsd_cleanup()` is now traced, to more easily see that the daemon is exiting (and/or start-up has aborted due to configuration or run-time issues). Warning about "world readable" files clarified. [#2417] + * sanity-check `NUMBER` values that they actually resolve as a `double` + or `long int`; report an un-flagged type as `STRING` if not (previously + blindly defaulted to `NUMBER` always). Debug-print presence of the + `IMMUTABLE` flag in `netget.c::get_type()`, and actually consider it + in the `netset.c::set_var()` method to abort early. Socket protocol + used between a driver and data server to exchange device state was + updated to pass `IMMUTABLE` flag, but this change should not impact + the NUT network protocol as published in RFC 9271. [#266] - nut-scanner: * the tool relies on dynamic loading of shared objects (library files) diff --git a/UPGRADING.adoc b/UPGRADING.adoc index 908ce287e9..35ebf31c3f 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -66,6 +66,19 @@ Changes from 2.8.2 to 2.8.3 and one structure (`nutscan_ipmi_t`) updated in a (hopefully) backwards compatible manner. [PR #2523, issue #2244 and numerous PRs for it] +- The `netset.c::set_var()` was updated to consider the `IMMUTABLE` flag on + values to reject writing into them quickly. This is currently expected + to only impact `override.*` values in vanilla NUT codebase, and should + not impact the NUT network protocol as published in RFC 9271 (only the + socket protocol used between a driver and data server to exchange device + state). [#266] + +- The `netset.c::get_type()` was updated to sanity-check `NUMBER`-flagged + values that the strings actually resolve into `double` or `long int` + values. Some codebase does not flag its values properly and warnings + can be emitted (to be reported for upstream NUT to fix these use-cases). + [#266] + - Internal API change for `sendsignalpid()` and `sendsignalfn()` methods, which can impact NUT forks which build using `libcommon.la` and similar libraries. Added new last argument with `const char *progname` (may be diff --git a/common/state.c b/common/state.c index 51eb8db6ff..aee3175df5 100644 --- a/common/state.c +++ b/common/state.c @@ -536,6 +536,11 @@ void state_setflags(st_tree_t *root, const char *var, size_t numflags, char **fl for (i = 0; i < numflags; i++) { + if (!strcasecmp(flag[i], "IMMUTABLE")) { + sttmp->flags |= ST_FLAG_IMMUTABLE; + continue; + } + if (!strcasecmp(flag[i], "RW")) { sttmp->flags |= ST_FLAG_RW; continue; diff --git a/docs/net-protocol.txt b/docs/net-protocol.txt index 577e29abaa..b7795c8de6 100644 --- a/docs/net-protocol.txt +++ b/docs/net-protocol.txt @@ -156,6 +156,11 @@ exponents, and comma for thousands separator are forbidden. For example: "1200.20" is valid, while "1,200.20" and "1200,20" and "1.2e4" are invalid. +Also note that internally NUT programs can flag variables as 'IMMUTABLE': +a value of such variable can not be changed after it was initially set +(this is used e.g. for "override" settings where the device is known to +provide bogus readings). The flag is not currently exposed with `GET` +commands, but can be a reason for a `SET` to fail. This replaces the old "VARTYPE" command. diff --git a/docs/nut.dict b/docs/nut.dict index 8a0c0b2b5b..d55e1313b9 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3197 utf-8 +personal_ws-1.1 en 3199 utf-8 AAC AAS ABI @@ -2380,8 +2380,10 @@ nd nds netcat netclient +netget netmask netserver +netset netsh netsnmp netvision diff --git a/docs/sock-protocol.txt b/docs/sock-protocol.txt index b92de60910..ca9b1f968a 100644 --- a/docs/sock-protocol.txt +++ b/docs/sock-protocol.txt @@ -112,7 +112,7 @@ flags are supported. Also note that they are not crammed together in This also replaces any previous flags for a given variable. -Currently supported flags include `RW`, `STRING` and `NUMBER` +Currently supported flags include `IMMUTABLE`, `RW`, `STRING` and `NUMBER` (detailed in the NUT Network Protocol documentation); unrecognized values are quietly ignored. diff --git a/drivers/dstate.c b/drivers/dstate.c index 59ef0c94d4..5ed9bae489 100644 --- a/drivers/dstate.c +++ b/drivers/dstate.c @@ -638,6 +638,9 @@ static int st_tree_dump_conn(st_tree_t *node, conn_t *conn) /* build the list */ snprintf(flist, sizeof(flist), "%s", node->var); + if (node->flags & ST_FLAG_IMMUTABLE) { + snprintfcat(flist, sizeof(flist), " IMMUTABLE"); + } if (node->flags & ST_FLAG_RW) { snprintfcat(flist, sizeof(flist), " RW"); } diff --git a/server/netget.c b/server/netget.c index 4be093cf36..93856a841a 100644 --- a/server/netget.c +++ b/server/netget.c @@ -24,6 +24,7 @@ #include "state.h" #include "desc.h" #include "neterr.h" +#include "nut_stdint.h" #include "netget.h" @@ -138,6 +139,19 @@ static void get_type(nut_ctype_t *client, const char *upsname, const char *var) snprintf(buf, sizeof(buf), "TYPE %s %s", upsname, var); + if (node->flags & ST_FLAG_IMMUTABLE) { +#if defined DEBUG && DEBUG + /* Properly exposing this needs also an update to + * docs/net-protocol.txt (promote the paragraph + * provided as a note currently) and to the NUT RFC + * https://www.rfc-editor.org/info/rfc9271 + */ + snprintfcat(buf, sizeof(buf), " IMMUTABLE"); +#endif + upsdebugx(3, "%s: UPS[%s] variable %s is IMMUTABLE", + __func__, upsname, var); + } + if (node->flags & ST_FLAG_RW) snprintfcat(buf, sizeof(buf), " RW"); @@ -162,6 +176,66 @@ static void get_type(nut_ctype_t *client, const char *upsname, const char *var) __func__, upsname, var); } + /* Sanity-check current contents */ + if (node->val && *(node->val)) { + double d; + long l; + int ok = 1; + size_t len = strlen(node->val); + + errno = 0; + if (!str_to_double_strict(node->val, &d, 10)) { + upsdebugx(3, "%s: UPS[%s] variable %s is flagged a NUMBER but not (exclusively) a double: %s", + __func__, upsname, var, node->val); + upsdebug_with_errno(4, "%s: val=%f len=%" PRIuSIZE, + __func__, d, len); + ok = 0; + } + + if (!ok) { + /* did not parse as a float... range issues or NaN? */ + errno = 0; + ok = 1; + if (!str_to_long_strict(node->val, &l, 10)) { + upsdebugx(3, "%s: UPS[%s] variable %s is flagged a NUMBER but not (exclusively) a long int: %s", + __func__, upsname, var, node->val); + upsdebug_with_errno(4, "%s: val=%ld len=%" PRIuSIZE, + __func__, l, len); + ok = 0; + } + } + +#if defined DEBUG && DEBUG + /* Need to figure out an "aux" value here (length of current + * string at least?) and propagate the flag into where netset + * would see it. Maybe this sanity-check should move into the + * core state.c logic, so dstate setting would already remember + * the defaulted flag (and maybe set another to clarify it is + * a guess). Currently that code does not concern itself with + * sanity-checks, it seems! + */ + if (!ok && !(node->flags & ST_FLAG_NUMBER)) { + upsdebugx(3, "%s: assuming UPS[%s] variable %s is a STRING after all, by contents; " + "value='%s' len='%" PRIuSIZE "' aux='%ld'", + __func__, upsname, var, node->val, len, node->aux); + + sendback(client, "%s STRING:%ld\n", buf, node->aux); + return; + } +#endif + + if (!ok) { + /* FIXME: Should this return an error? + * Value was explicitly flagged as a NUMBER but is not by content. + * Note that state_addinfo() does not sanity-check; but + * netset.c::set_var() does though (for protocol clients). + */ + upslogx(LOG_WARNING, "%s: UPS[%s] variable %s is flagged as a NUMBER but is not " + "by contents (please report as a bug to NUT project)): %s", + __func__, upsname, var, node->val); + } + } + sendback(client, "%s NUMBER\n", buf); } diff --git a/server/netset.c b/server/netset.c index bd5f033a2e..b35559572d 100644 --- a/server/netset.c +++ b/server/netset.c @@ -152,6 +152,16 @@ static void set_var(nut_ctype_t *client, const char *upsname, const char *var, } } + /* FIXME: Consider null/non-null values for strings + * See note on sstate_getnode() idea above. + */ + if (sstate_getflags(ups, var) & ST_FLAG_IMMUTABLE) { + upsdebugx(3, "%s: UPS [%s]: value of %s is already set and immutable", + __func__, ups->name, var); + send_err(client, NUT_ERR_READONLY); + return; + } + /* must be OK now */ snprintf(cmd, sizeof(cmd), "SET %s \"%s\"",