[tor-commits] [tor/master] confmgt: when a units value is invalid, include a meaningful error.
nickm at torproject.org
nickm at torproject.org
Thu Mar 5 13:48:47 UTC 2020
commit 7e7aff9b6a3bcf240fe30320e1bcd5ffdab92f13
Author: Nick Mathewson <nickm at torproject.org>
Date: Wed Feb 26 12:40:53 2020 -0500
confmgt: when a units value is invalid, include a meaningful error.
Part of 33460.
---
src/lib/confmgt/type_defs.c | 20 ++++++++++++++++----
src/lib/confmgt/unitparse.c | 45 ++++++++++++++++++++++++++++++++-------------
src/lib/confmgt/unitparse.h | 3 ++-
src/test/test_options.c | 16 ++++------------
4 files changed, 54 insertions(+), 30 deletions(-)
diff --git a/src/lib/confmgt/type_defs.c b/src/lib/confmgt/type_defs.c
index dd0a846f3..d9e5e1e4c 100644
--- a/src/lib/confmgt/type_defs.c
+++ b/src/lib/confmgt/type_defs.c
@@ -229,11 +229,17 @@ units_parse_u64(void *target, const char *value, char **errmsg,
tor_assert(table);
uint64_t *v = (uint64_t*)target;
int ok=1;
- *v = config_parse_units(value, table, &ok);
+ char *msg = NULL;
+ *v = config_parse_units(value, table, &ok, &msg);
if (!ok) {
- *errmsg = tor_strdup("Provided value is malformed or out of bounds.");
+ tor_asprintf(errmsg, "Provided value is malformed or out of bounds: %s",
+ msg);
+ tor_free(msg);
return -1;
}
+ if (BUG(msg)) {
+ tor_free(msg);
+ }
return 0;
}
@@ -245,11 +251,17 @@ units_parse_int(void *target, const char *value, char **errmsg,
tor_assert(table);
int *v = (int*)target;
int ok=1;
- uint64_t u64 = config_parse_units(value, table, &ok);
+ char *msg = NULL;
+ uint64_t u64 = config_parse_units(value, table, &ok, &msg);
if (!ok) {
- *errmsg = tor_strdup("Provided value is malformed or out of bounds.");
+ tor_asprintf(errmsg, "Provided value is malformed or out of bounds: %s",
+ msg);
+ tor_free(msg);
return -1;
}
+ if (BUG(msg)) {
+ tor_free(msg);
+ }
if (u64 > INT_MAX) {
tor_asprintf(errmsg, "Provided value %s is too large", value);
return -1;
diff --git a/src/lib/confmgt/unitparse.c b/src/lib/confmgt/unitparse.c
index 52c0eabb6..61edc6069 100644
--- a/src/lib/confmgt/unitparse.c
+++ b/src/lib/confmgt/unitparse.c
@@ -13,7 +13,9 @@
#include "lib/confmgt/unitparse.h"
#include "lib/log/log.h"
#include "lib/log/util_bug.h"
+#include "lib/malloc/malloc.h"
#include "lib/string/parse_int.h"
+#include "lib/string/printf.h"
#include "lib/string/util_string.h"
#include "lib/intmath/muldiv.h"
@@ -110,23 +112,30 @@ const struct unit_table_t time_msec_units[] = {
* table <b>u</b>, then multiply the number by the unit multiplier.
* On success, set *<b>ok</b> to 1 and return this product.
* Otherwise, set *<b>ok</b> to 0.
- * Warns user when overflow or a negative value is detected.
+ *
+ * If an error (like overflow or a negative value is detected), put an error
+ * message in *<b>errmsg_out</b> if that pointer is non-NULL, and otherwise
+ * log a warning.
*/
uint64_t
-config_parse_units(const char *val, const unit_table_t *u, int *ok)
+config_parse_units(const char *val, const unit_table_t *u, int *ok,
+ char **errmsg_out)
{
uint64_t v = 0;
double d = 0;
int use_float = 0;
char *cp;
+ char *errmsg = NULL;
tor_assert(ok);
v = tor_parse_uint64(val, 10, 0, UINT64_MAX, ok, &cp);
if (!*ok || (cp && *cp == '.')) {
d = tor_parse_double(val, 0, (double)UINT64_MAX, ok, &cp);
- if (!*ok)
+ if (!*ok) {
+ tor_asprintf(&errmsg, "Unable to parse %s as a number", val);
goto done;
+ }
use_float = 1;
}
@@ -148,8 +157,8 @@ config_parse_units(const char *val, const unit_table_t *u, int *ok)
d = u->multiplier * d;
if (d < 0) {
- log_warn(LD_CONFIG, "Got a negative value while parsing %s %s",
- val, u->unit);
+ tor_asprintf(&errmsg, "Got a negative value while parsing %s %s",
+ val, u->unit);
*ok = 0;
goto done;
}
@@ -157,8 +166,8 @@ config_parse_units(const char *val, const unit_table_t *u, int *ok)
// Some compilers may warn about casting a double to an unsigned type
// because they don't know if d is >= 0
if (d >= 0 && (d > (double)INT64_MAX || (uint64_t)d > INT64_MAX)) {
- log_warn(LD_CONFIG, "Overflow detected while parsing %s %s",
- val, u->unit);
+ tor_asprintf(&errmsg, "Overflow while parsing %s %s",
+ val, u->unit);
*ok = 0;
goto done;
}
@@ -168,8 +177,8 @@ config_parse_units(const char *val, const unit_table_t *u, int *ok)
v = tor_mul_u64_nowrap(v, u->multiplier);
if (v > INT64_MAX) {
- log_warn(LD_CONFIG, "Overflow detected while parsing %s %s",
- val, u->unit);
+ tor_asprintf(&errmsg, "Overflow while parsing %s %s",
+ val, u->unit);
*ok = 0;
goto done;
}
@@ -179,10 +188,20 @@ config_parse_units(const char *val, const unit_table_t *u, int *ok)
goto done;
}
}
- log_warn(LD_CONFIG, "Unknown unit '%s'.", cp);
+ tor_asprintf(&errmsg, "Unknown unit in %s", val);
*ok = 0;
done:
+ if (errmsg) {
+ tor_assert_nonfatal(!*ok);
+ if (errmsg_out) {
+ *errmsg_out = errmsg;
+ } else {
+ log_warn(LD_CONFIG, "%s", errmsg);
+ tor_free(errmsg);
+ }
+ }
+
if (*ok)
return v;
else
@@ -196,7 +215,7 @@ config_parse_units(const char *val, const unit_table_t *u, int *ok)
uint64_t
config_parse_memunit(const char *s, int *ok)
{
- uint64_t u = config_parse_units(s, memory_units, ok);
+ uint64_t u = config_parse_units(s, memory_units, ok, NULL);
return u;
}
@@ -208,7 +227,7 @@ int
config_parse_msec_interval(const char *s, int *ok)
{
uint64_t r;
- r = config_parse_units(s, time_msec_units, ok);
+ r = config_parse_units(s, time_msec_units, ok, NULL);
if (r > INT_MAX) {
log_warn(LD_CONFIG, "Msec interval '%s' is too long", s);
*ok = 0;
@@ -225,7 +244,7 @@ int
config_parse_interval(const char *s, int *ok)
{
uint64_t r;
- r = config_parse_units(s, time_units, ok);
+ r = config_parse_units(s, time_units, ok, NULL);
if (r > INT_MAX) {
log_warn(LD_CONFIG, "Interval '%s' is too long", s);
*ok = 0;
diff --git a/src/lib/confmgt/unitparse.h b/src/lib/confmgt/unitparse.h
index 3f4f039a0..047e11b42 100644
--- a/src/lib/confmgt/unitparse.h
+++ b/src/lib/confmgt/unitparse.h
@@ -25,7 +25,8 @@ extern const unit_table_t memory_units[];
extern const unit_table_t time_units[];
extern const struct unit_table_t time_msec_units[];
-uint64_t config_parse_units(const char *val, const unit_table_t *u, int *ok);
+uint64_t config_parse_units(const char *val, const unit_table_t *u, int *ok,
+ char **errmsg_out);
uint64_t config_parse_memunit(const char *s, int *ok);
int config_parse_msec_interval(const char *s, int *ok);
diff --git a/src/test/test_options.c b/src/test/test_options.c
index b67c37f50..129a8299e 100644
--- a/src/test/test_options.c
+++ b/src/test/test_options.c
@@ -307,18 +307,10 @@ test_options_validate(void *arg)
WANT_ERR("BridgeRelay 1\nDirCache 0",
"We're a bridge but DirCache is disabled.", PH_VALIDATE);
- // XXXX We should replace this with a more full error message once #29211
- // XXXX is done. It is truncated for now because at the current stage
- // XXXX of refactoring, we can't give a full error message like before.
- WANT_ERR_LOG("HeartbeatPeriod 21 snarks",
- "malformed or out of bounds", LOG_WARN,
- "Unknown unit 'snarks'.",
- PH_ASSIGN);
- // XXXX As above.
- WANT_ERR_LOG("LogTimeGranularity 21 snarks",
- "malformed or out of bounds", LOG_WARN,
- "Unknown unit 'snarks'.",
- PH_ASSIGN);
+ WANT_ERR("HeartbeatPeriod 21 snarks",
+ "Unknown unit in 21 snarks", PH_ASSIGN);
+ WANT_ERR("LogTimeGranularity 21 snarks",
+ "Unknown unit in 21 snarks", PH_ASSIGN);
OK("HeartbeatPeriod 1 hour", PH_VALIDATE);
OK("LogTimeGranularity 100 milliseconds", PH_VALIDATE);
More information about the tor-commits
mailing list