From 71232aac41bf7bf5dd309e8d61e9771ed985f206 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 23 Feb 2018 16:49:17 +0100 Subject: [PATCH 1/5] rules: add a missing comma in 70-uaccess.rules since it improves readability rule-syntax-check.py failed with the following error: $ ./test/rule-syntax-check.py ./src/login/70-uaccess.rules Invalid line ./src/login/70-uaccess.rules:31: SUBSYSTEM=="sound", TAG+="uaccess" OPTIONS+="static_node=snd/timer", OPTIONS+="static_node=snd/seq" clause: TAG+="uaccess" OPTIONS+="static_node=snd/timer" The comma is actually optional but the script makes it mandatory which seems a good thing since it improves readability. --- src/login/70-uaccess.rules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/login/70-uaccess.rules b/src/login/70-uaccess.rules index f2c838f3535..3515d292ac5 100644 --- a/src/login/70-uaccess.rules +++ b/src/login/70-uaccess.rules @@ -27,7 +27,7 @@ SUBSYSTEM=="block", ENV{ID_CDROM}=="1", TAG+="uaccess" SUBSYSTEM=="scsi_generic", SUBSYSTEMS=="scsi", ATTRS{type}=="4|5", TAG+="uaccess" # Sound devices -SUBSYSTEM=="sound", TAG+="uaccess" \ +SUBSYSTEM=="sound", TAG+="uaccess", \ OPTIONS+="static_node=snd/timer", OPTIONS+="static_node=snd/seq" # ffado is an userspace driver for firewire sound cards From 905ca72a8fc174be0d012c2c87fa6f0437bc9790 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 23 Feb 2018 16:54:40 +0100 Subject: [PATCH 2/5] rule-syntax-check: PROGRAM is not supposed to get value assigned In udev man page, "PROGRAM" key is part of the keys which are used for matching purposes so it should only be used with the compare operator "==". Actually it doesn't really make sense to assign it a value. udev code allows both "=" and "==" for PROGRAM and both are handled the same way but for consistencies it's better to have only the compare operator allowed by the rule syntax checker. No rules shipped by systemd use PROGRAM key so nothing need to be changed in our rule files. --- test/rule-syntax-check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/rule-syntax-check.py b/test/rule-syntax-check.py index e053b027ca1..f24285ddac6 100755 --- a/test/rule-syntax-check.py +++ b/test/rule-syntax-check.py @@ -28,9 +28,9 @@ rules_files = sys.argv[1:] if not rules_files: sys.exit('Specify files to test as arguments') -no_args_tests = re.compile(r'(ACTION|DEVPATH|KERNELS?|NAME|SYMLINK|SUBSYSTEMS?|DRIVERS?|TAG|RESULT|TEST)\s*(?:=|!)=\s*"([^"]*)"$') +no_args_tests = re.compile(r'(ACTION|DEVPATH|KERNELS?|NAME|SYMLINK|SUBSYSTEMS?|DRIVERS?|TAG|PROGRAM|RESULT|TEST)\s*(?:=|!)=\s*"([^"]*)"$') args_tests = re.compile(r'(ATTRS?|ENV|TEST){([a-zA-Z0-9/_.*%-]+)}\s*(?:=|!)=\s*"([^"]*)"$') -no_args_assign = re.compile(r'(NAME|SYMLINK|OWNER|GROUP|MODE|TAG|PROGRAM|RUN|LABEL|GOTO|OPTIONS|IMPORT)\s*(?:\+=|:=|=)\s*"([^"]*)"$') +no_args_assign = re.compile(r'(NAME|SYMLINK|OWNER|GROUP|MODE|TAG|RUN|LABEL|GOTO|OPTIONS|IMPORT)\s*(?:\+=|:=|=)\s*"([^"]*)"$') args_assign = re.compile(r'(ATTR|ENV|IMPORT|RUN){([a-zA-Z0-9/_.*%-]+)}\s*(=|\+=)\s*"([^"]*)"$') result = 0 From 75a56cb63259505e568d0524461d446040ceba6c Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 23 Feb 2018 17:12:50 +0100 Subject: [PATCH 3/5] rule-syntax-check: values can contain escaped double quotes This is true since commit 7e760b79ad143b26a5c937afa7666a7c40508f85. Note that the changes in the regex expressions relies on the fact that the script assumes that the comma separator is mandatory. Add a comment in the script to clarify this. --- test/rule-syntax-check.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/rule-syntax-check.py b/test/rule-syntax-check.py index f24285ddac6..dcbab5fc6dc 100755 --- a/test/rule-syntax-check.py +++ b/test/rule-syntax-check.py @@ -28,10 +28,10 @@ rules_files = sys.argv[1:] if not rules_files: sys.exit('Specify files to test as arguments') -no_args_tests = re.compile(r'(ACTION|DEVPATH|KERNELS?|NAME|SYMLINK|SUBSYSTEMS?|DRIVERS?|TAG|PROGRAM|RESULT|TEST)\s*(?:=|!)=\s*"([^"]*)"$') -args_tests = re.compile(r'(ATTRS?|ENV|TEST){([a-zA-Z0-9/_.*%-]+)}\s*(?:=|!)=\s*"([^"]*)"$') -no_args_assign = re.compile(r'(NAME|SYMLINK|OWNER|GROUP|MODE|TAG|RUN|LABEL|GOTO|OPTIONS|IMPORT)\s*(?:\+=|:=|=)\s*"([^"]*)"$') -args_assign = re.compile(r'(ATTR|ENV|IMPORT|RUN){([a-zA-Z0-9/_.*%-]+)}\s*(=|\+=)\s*"([^"]*)"$') +no_args_tests = re.compile(r'(ACTION|DEVPATH|KERNELS?|NAME|SYMLINK|SUBSYSTEMS?|DRIVERS?|TAG|PROGRAM|RESULT|TEST)\s*(?:=|!)=\s*"(.*)"$') +args_tests = re.compile(r'(ATTRS?|ENV|TEST){([a-zA-Z0-9/_.*%-]+)}\s*(?:=|!)=\s*"(.*)"$') +no_args_assign = re.compile(r'(NAME|SYMLINK|OWNER|GROUP|MODE|TAG|RUN|LABEL|GOTO|OPTIONS|IMPORT)\s*(?:\+=|:=|=)\s*"(.*)"$') +args_assign = re.compile(r'(ATTR|ENV|IMPORT|RUN){([a-zA-Z0-9/_.*%-]+)}\s*(=|\+=)\s*"(.*)"$') result = 0 buffer = '' @@ -54,6 +54,8 @@ for path in rules_files: if not line or line.startswith('#'): continue + # Separator ',' is normally optional but we make it mandatory here as + # it generally improves the readability of the rules. for clause in line.split(','): clause = clause.strip() if not (no_args_tests.match(clause) or args_tests.match(clause) or From d498347a01266fc77ad9086611495802096d00f6 Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Tue, 27 Feb 2018 11:12:18 -0800 Subject: [PATCH 4/5] rule-syntax-check: add support for escaped double quotes Add support to backslash-escaped double quote inside a string. Tested by modifying src/login/70-uaccess.rules to include: ACTION=="remove" it", GOTO="uaccess_end" And had the rule checker complain about it: $ test/rule-syntax-check.py src/login/70-uaccess.rules # looking at src/login/70-uaccess.rules Invalid line src/login/70-uaccess.rules:10: ACTION=="remove" it", GOTO="uaccess_end" clause: ACTION=="remove" it" --- test/rule-syntax-check.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/rule-syntax-check.py b/test/rule-syntax-check.py index dcbab5fc6dc..1a58d17328e 100755 --- a/test/rule-syntax-check.py +++ b/test/rule-syntax-check.py @@ -28,10 +28,11 @@ rules_files = sys.argv[1:] if not rules_files: sys.exit('Specify files to test as arguments') -no_args_tests = re.compile(r'(ACTION|DEVPATH|KERNELS?|NAME|SYMLINK|SUBSYSTEMS?|DRIVERS?|TAG|PROGRAM|RESULT|TEST)\s*(?:=|!)=\s*"(.*)"$') -args_tests = re.compile(r'(ATTRS?|ENV|TEST){([a-zA-Z0-9/_.*%-]+)}\s*(?:=|!)=\s*"(.*)"$') -no_args_assign = re.compile(r'(NAME|SYMLINK|OWNER|GROUP|MODE|TAG|RUN|LABEL|GOTO|OPTIONS|IMPORT)\s*(?:\+=|:=|=)\s*"(.*)"$') -args_assign = re.compile(r'(ATTR|ENV|IMPORT|RUN){([a-zA-Z0-9/_.*%-]+)}\s*(=|\+=)\s*"(.*)"$') +quoted_string_re = r'"(?:[^\\"]|\\.)*"' +no_args_tests = re.compile(r'(ACTION|DEVPATH|KERNELS?|NAME|SYMLINK|SUBSYSTEMS?|DRIVERS?|TAG|PROGRAM|RESULT|TEST)\s*(?:=|!)=\s*' + quoted_string_re + '$') +args_tests = re.compile(r'(ATTRS?|ENV|TEST){([a-zA-Z0-9/_.*%-]+)}\s*(?:=|!)=\s*' + quoted_string_re + '$') +no_args_assign = re.compile(r'(NAME|SYMLINK|OWNER|GROUP|MODE|TAG|RUN|LABEL|GOTO|OPTIONS|IMPORT)\s*(?:\+=|:=|=)\s*' + quoted_string_re + '$') +args_assign = re.compile(r'(ATTR|ENV|IMPORT|RUN){([a-zA-Z0-9/_.*%-]+)}\s*(=|\+=)\s*' + quoted_string_re + '$') result = 0 buffer = '' From c9715ffce347310a5196d1375810d41fbdd01fd6 Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Tue, 27 Feb 2018 13:11:07 -0800 Subject: [PATCH 5/5] rule-syntax-check: allow commas inside quoted strings Using a regex to match the groups is smarter than the split(',') that would break in those cases. Tested: SUBSYSTEM=="usb", ENV{ID_USB_INTERFACES}=="*:060101:*,*:070202:*", TAG+="uaccess" Rule checker doesn't break there after this commit. --- test/rule-syntax-check.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/rule-syntax-check.py b/test/rule-syntax-check.py index 1a58d17328e..7ee34eb7002 100755 --- a/test/rule-syntax-check.py +++ b/test/rule-syntax-check.py @@ -33,6 +33,8 @@ no_args_tests = re.compile(r'(ACTION|DEVPATH|KERNELS?|NAME|SYMLINK|SUBSYSTEMS?|D args_tests = re.compile(r'(ATTRS?|ENV|TEST){([a-zA-Z0-9/_.*%-]+)}\s*(?:=|!)=\s*' + quoted_string_re + '$') no_args_assign = re.compile(r'(NAME|SYMLINK|OWNER|GROUP|MODE|TAG|RUN|LABEL|GOTO|OPTIONS|IMPORT)\s*(?:\+=|:=|=)\s*' + quoted_string_re + '$') args_assign = re.compile(r'(ATTR|ENV|IMPORT|RUN){([a-zA-Z0-9/_.*%-]+)}\s*(=|\+=)\s*' + quoted_string_re + '$') +# Find comma-separated groups, but allow commas that are inside quoted strings. +comma_separated_group_re = re.compile(r'(?:[^,"]|' + quoted_string_re + ')+') result = 0 buffer = '' @@ -57,8 +59,8 @@ for path in rules_files: # Separator ',' is normally optional but we make it mandatory here as # it generally improves the readability of the rules. - for clause in line.split(','): - clause = clause.strip() + for clause_match in comma_separated_group_re.finditer(line): + clause = clause_match.group().strip() if not (no_args_tests.match(clause) or args_tests.match(clause) or no_args_assign.match(clause) or args_assign.match(clause)):