From acc1954a03940b34aab4b814248e9ddf7e49a901 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 23 Mar 2023 08:00:00 +0000 Subject: [PATCH] udev-rules: check token delimiters When udev_rules_parse_file() is called by udevadm verify, issue warnings about the following conditions in udev rules: * the first token in the rule is preceded with a comma * the last token in the rule is followed by a comma * there is no comma between tokens * there is no whitespace between tokens * there is more than a single comma between tokens * there is whitespace between a token and a comma * there is no whitespace after comma --- src/udev/udev-rules.c | 52 +++++++++++++++++++++++++++++++++-- test/units/testsuite-17.11.sh | 31 +++++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 23dd31229d7..ecf59893128 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1069,6 +1069,51 @@ static UdevRuleOperatorType parse_operator(const char *op) { return _OP_TYPE_INVALID; } +static void check_token_delimiters(UdevRuleLine *rule_line, const char *line) { + assert(rule_line); + + size_t n_comma = 0; + bool ws_before_comma = false, ws_after_comma = false; + const char *p; + + for (p = line; !isempty(p); ++p) { + if (*p == ',') + ++n_comma; + else if (strchr(WHITESPACE, *p)) { + if (n_comma > 0) + ws_after_comma = true; + else + ws_before_comma = true; + } else + break; + } + + if (line == rule_line->line) { + /* this is the first token of the rule */ + if (n_comma > 0) + log_line_warning(rule_line, "Stray leading comma."); + } else if (isempty(p)) { + /* there are no more tokens in the rule */ + if (n_comma > 0) + log_line_warning(rule_line, "Stray trailing comma."); + } else { + /* single comma is expected */ + if (n_comma == 0) + log_line_warning(rule_line, "A comma between tokens is expected."); + else if (n_comma > 1) + log_line_warning(rule_line, "More than one comma between tokens."); + + /* whitespace after comma is expected */ + if (n_comma > 0) { + if (ws_before_comma) + log_line_warning(rule_line, "Stray whitespace before comma."); + if (!ws_after_comma) + log_line_warning(rule_line, "Whitespace after comma is expected."); + } else if (!ws_before_comma && !ws_after_comma) + log_line_warning(rule_line, "Whitespace between tokens is expected."); + } +} + static int parse_line(char **line, char **ret_key, char **ret_attr, UdevRuleOperatorType *ret_op, char **ret_value) { char *key_begin, *key_end, *attr, *tmp; UdevRuleOperatorType op; @@ -1140,7 +1185,7 @@ static void sort_tokens(UdevRuleLine *rule_line) { } } -static int rule_add_line(UdevRuleFile *rule_file, const char *line_str, unsigned line_nr) { +static int rule_add_line(UdevRuleFile *rule_file, const char *line_str, unsigned line_nr, bool extra_checks) { _cleanup_(udev_rule_line_freep) UdevRuleLine *rule_line = NULL; _cleanup_free_ char *line = NULL; char *p; @@ -1172,6 +1217,9 @@ static int rule_add_line(UdevRuleFile *rule_file, const char *line_str, unsigned char *key, *attr, *value; UdevRuleOperatorType op; + if (extra_checks) + check_token_delimiters(rule_line, p); + r = parse_line(&p, &key, &attr, &op, &value); if (r < 0) return log_line_error_errno(rule_line, r, "Invalid key/value pair, ignoring."); @@ -1457,7 +1505,7 @@ int udev_rules_parse_file(UdevRules *rules, const char *filename, bool extra_che if (ignore_line) log_file_error(rule_file, line_nr, "Line is too long, ignored"); else if (len > 0) - (void) rule_add_line(rule_file, line, line_nr); + (void) rule_add_line(rule_file, line, line_nr, extra_checks); continuation = mfree(continuation); ignore_line = false; diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index 19ae35ce8f7..c7c793e71ba 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -287,6 +287,15 @@ test_syntax_error 'KERNEL=="|a|b", KERNEL=="b|a|", NAME="c"' 'duplicate expressi # shellcheck disable=SC2016 test_syntax_error 'ENV{DISKSEQ}=="?*", ENV{DEVTYPE}!="partition", ENV{DISKSEQ}=="?*", ENV{ID_IGNORE_DISKSEQ}!="1", SYMLINK+="disk/by-diskseq/$env{DISKSEQ}"' \ 'duplicate expressions' +test_syntax_error ',ACTION=="a", NAME="b"' 'Stray leading comma.' +test_syntax_error ' ,ACTION=="a", NAME="b"' 'Stray leading comma.' +test_syntax_error ', ACTION=="a", NAME="b"' 'Stray leading comma.' +test_syntax_error 'ACTION=="a", NAME="b",' 'Stray trailing comma.' +test_syntax_error 'ACTION=="a", NAME="b", ' 'Stray trailing comma.' +test_syntax_error 'ACTION=="a" NAME="b"' 'A comma between tokens is expected.' +test_syntax_error 'ACTION=="a",, NAME="b"' 'More than one comma between tokens.' +test_syntax_error 'ACTION=="a" , NAME="b"' 'Stray whitespace before comma.' +test_syntax_error 'ACTION=="a",NAME="b"' 'Whitespace after comma is expected.' cat >"${rules}" <<'EOF' KERNEL=="a|b", KERNEL=="a|c", NAME="d" @@ -348,6 +357,28 @@ EOF cp "${workdir}/default_output_1_fail" "${exo}" assert_1 "${rules}" +cat >"${rules}" <<'EOF' +ACTION=="a"NAME="b" +EOF +cat >"${exp}" <"${rules}" <<'EOF' +ACTION=="a" ,NAME="b" +EOF +cat >"${exp}" <"${workdir}/${exp}" cd -