Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CLN service startup failure by trimming trailing spaces in config parameters #7251

Merged
merged 2 commits into from
Aug 10, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion common/configvar.c
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@ const struct opt_table *configvar_unparsed(struct configvar *cv)
ot = opt_find_short(cv->configline[0]);
cv->optarg = NULL;
} else {
ot = opt_find_long(cv->configline, &cv->optarg);
ot = opt_find_long(cv->configline, cast_const2(const char **, &cv->optarg));
}
if (!ot)
return NULL;
@@ -51,6 +51,15 @@ const struct opt_table *configvar_unparsed(struct configvar *cv)
return ot;
}

static void trim_whitespace(char *s)
{
size_t len = strlen(s);

while (len > 0 && cisspace(s[len - 1]))
len--;
s[len] = '\0';
}

const char *configvar_parse(struct configvar *cv,
bool early,
bool full_knowledge,
@@ -82,6 +91,8 @@ const char *configvar_parse(struct configvar *cv,
} else {
if (!cv->optarg)
return "requires an argument";
if (!(ot->type & OPT_KEEP_WHITESPACE))
trim_whitespace(cv->optarg);
return ot->cb_arg(cv->optarg, ot->u.arg);
}
}
6 changes: 4 additions & 2 deletions common/configvar.h
Original file line number Diff line number Diff line change
@@ -35,13 +35,13 @@ struct configvar {
/* Where did we get this from? */
enum configvar_src src;
/* Never NULL, the whole line */
const char *configline;
char *configline;

/* These are filled in by configvar_parse */
/* The variable name (without any =) */
const char *optvar;
/* NULL for no-arg options, otherwise points after =. */
const char *optarg;
char *optarg;
/* Was this overridden by a following option? */
bool overridden;
};
@@ -60,6 +60,8 @@ struct configvar {
#define OPT_SHOWBOOL (1 << (OPT_USER_START+5))
/* Can be changed at runtime: cb will get called with NULL for `check`! */
#define OPT_DYNAMIC (1 << (OPT_USER_START+6))
/* Keep whitespace at the end of the option argument */
#define OPT_KEEP_WHITESPACE (1 << (OPT_USER_START+7))

/* Use this instead of opt_register_*_arg if you want OPT_* from above */
#define clnopt_witharg(names, type, cb, show, arg, desc) \
8 changes: 8 additions & 0 deletions doc/lightningd-config.5.md
Original file line number Diff line number Diff line change
@@ -83,6 +83,14 @@ re-enable it. Unless we've made a horrible mistake it's probably time
to complain or fix to whatever is using the old API. It can be
specified multiple times for different features.

### Whitespace Handling

Because it's a common error, we automatically trim whitespace from the
end of most configuration options. Exceptions are noted below:

- `log-prefix`: Preserves whitespace at the end.
- `alias`: Preserves whitespace at the end.

### Bitcoin control options:

Bitcoin control options:
5 changes: 2 additions & 3 deletions lightningd/log.c
Original file line number Diff line number Diff line change
@@ -915,9 +915,8 @@ void opt_register_logging(struct lightningd *ld)
opt_set_bool_arg, opt_show_bool,
&ld->log_book->print_timestamps,
"prefix log messages with timestamp");
opt_register_early_arg("--log-prefix", arg_log_prefix, show_log_prefix,
ld->log_book,
"log prefix");
clnopt_witharg("--log-prefix", OPT_EARLY|OPT_KEEP_WHITESPACE,
arg_log_prefix, show_log_prefix, ld->log_book, "log prefix");
clnopt_witharg("--log-file=<file>",
OPT_EARLY|OPT_MULTI,
arg_log_to_file, NULL, ld,
11 changes: 5 additions & 6 deletions lightningd/options.c
Original file line number Diff line number Diff line change
@@ -1525,9 +1525,8 @@ static void register_opts(struct lightningd *ld)
opt_lightningd_usage, ld, "Print this message.");
opt_register_arg("--rgb", opt_set_rgb, opt_show_rgb, ld,
"RRGGBB hex color for node");
opt_register_arg("--alias", opt_set_alias, opt_show_alias, ld,
"Up to 32-byte alias for node");

clnopt_witharg("--alias", OPT_KEEP_WHITESPACE, opt_set_alias,
opt_show_alias, ld, "Up to 32-byte alias for node");
opt_register_arg("--pid-file=<file>", opt_set_talstr, opt_show_charp,
&ld->pidfile,
"Specify pid file");
@@ -1888,9 +1887,9 @@ void handle_early_opts(struct lightningd *ld, int argc, char *argv[])
}

/* Free *str, set *str to copy with `cln` prepended */
static void prefix_cln(const char **str STEALS)
static void prefix_cln(char **str STEALS)
{
const char *newstr = tal_fmt(tal_parent(*str), "cln%s", *str);
char *newstr = tal_fmt(tal_parent(*str), "cln%s", *str);
tal_free(*str);
*str = newstr;
}
@@ -1911,7 +1910,7 @@ static void fixup_clnrest_options(struct lightningd *ld)
&& !strstarts(cv->configline, "rest-certs="))
continue;
/* Did some (plugin) claim it? */
if (opt_find_long(cv->configline, &cv->optarg))
if (opt_find_long(cv->configline, cast_const2(const char **, &cv->optarg)))
continue;
if (!opt_deprecated_ok(ld,
tal_strndup(tmpctx, cv->configline,
37 changes: 37 additions & 0 deletions tests/test_misc.py
Original file line number Diff line number Diff line change
@@ -3867,6 +3867,43 @@ def test_fast_shutdown(node_factory):
break


def test_config_whitespace(node_factory):
""" Test the configuration parsing with extra
whitespace in the configuration file. """
l1 = node_factory.get_node()

configfile = os.path.join(l1.daemon.opts.get("lightning-dir"), TEST_NETWORK, 'config')

# Stop the node to modify the configuration file safely
l1.stop()

# Ensure the log-prefix option is not set in the command line arguments
if 'log-prefix' in l1.daemon.opts:
del l1.daemon.opts['log-prefix']

# Write configuration parameters with extra whitespace
with open(configfile, "a") as f:
f.write("\n\n# Test whitespace\n")
f.write("funder-policy-mod=100 \n")
f.write("funder-min-their-funding=10000\n")
f.write("allow-deprecated-apis=false \n")
f.write("alias=MyLightningNode \n")
f.write("log-prefix=MyNode \n")

l1.start()

configs = l1.rpc.listconfigs()

# Verify that the trimmed configuration values are correctly set
assert configs['configs']['funder-policy-mod']['value_str'] == '100', "funder-policy-mod should be '100'"
assert configs['configs']['funder-min-their-funding']['value_str'] == '10000', "funder-min-their-funding should be '10000'"
assert configs['configs']['allow-deprecated-apis']['value_bool'] is False, "allow-deprecated-apis should be False"

# We want to keep the whitespaces at the parameter 'alias' & 'log-prefix'
assert configs['configs']['alias']['value_str'] == 'MyLightningNode ', "alias should be 'MyLightningNode '"
assert configs['configs']['log-prefix']['value_str'] == 'MyNode ', "log-prefix should be 'MyNode '"


def test_setconfig(node_factory, bitcoind):
l1, l2 = node_factory.line_graph(2, fundchannel=False)
configfile = os.path.join(l2.daemon.opts.get("lightning-dir"), TEST_NETWORK, 'config')