Message ID | 20250203164235.1503956-2-n.merinov@inango-systems.com |
---|---|
State | Accepted, archived |
Commit | 93059aad13a12cd69d86368795c88e5349197d5d |
Headers | show |
Series | [v2] parse: Forbid ambiguous assignments to ${.}, ${+}, and ${:} variables | expand |
On Mon, 2025-02-03 at 18:42 +0200, Nikolai Merinov wrote: > Old code that parse variable names in assignment commands behave differently for > variables that ends with special symbol for single-character variable names and > multi-character variable names. For example: > > A+="1" # Change variable ${A}, '+' glued to '=' > A+ = "1" # Change variable ${A+} > > +="1" # Change variable ${+}, the '+' symbol not part of assignment operator > + = "1" # Change variable ${+} > > New code would always assume that '.=', '+=', and ':=' is assignment operator. > As result code like the following would raise parsing error > > +="value" > > While code with extra spaces would work as before > > + = "value" # Change variable ${+} > > This change allow to catch issues in code that generate bitbake configuration > files in a manner like "echo ${VARNAME}+=${VALUE} >> conf/local.conf" > > Signed-off-by: Nikolai Merinov <n.merinov@inango-systems.com> > --- > lib/bb/parse/parse_py/ConfHandler.py | 4 +++- > lib/bb/tests/parse.py | 20 ++++++++++++++++++++ > 2 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/lib/bb/parse/parse_py/ConfHandler.py b/lib/bb/parse/parse_py/ConfHandler.py > index 24f81f7e9..1bde59725 100644 > --- a/lib/bb/parse/parse_py/ConfHandler.py > +++ b/lib/bb/parse/parse_py/ConfHandler.py > @@ -20,7 +20,7 @@ from bb.parse import ParseError, resolve_file, ast, logger, handle > __config_regexp__ = re.compile( r""" > ^ > (?P<exp>export\s+)? > - (?P<var>[a-zA-Z0-9\-_+.${}/~:]+?) > + (?P<var>[a-zA-Z0-9\-_+.${}/~:]*?) > (\[(?P<flag>[a-zA-Z0-9\-_+.][a-zA-Z0-9\-_+.@/]*)\])? > > \s* ( > @@ -166,6 +166,8 @@ def feeder(lineno, s, fn, statements, baseconfig=False, conffile=True): > m = __config_regexp__.match(s) > if m: > groupd = m.groupdict() > + if groupd['var'] == "": > + raise ParseError("Empty variable name in assignment: '%s'" % s, fn, lineno); > ast.handleData(statements, fn, lineno, groupd) > return > > diff --git a/lib/bb/tests/parse.py b/lib/bb/tests/parse.py > index cb60af364..7598d5904 100644 > --- a/lib/bb/tests/parse.py > +++ b/lib/bb/tests/parse.py > @@ -443,3 +443,23 @@ include \\ > in_file.write("\n".join(lines)) > in_file.flush() > bb.parse.handle(recipename_closed, bb.data.createCopy(self.d)) > + > + special_character_assignment = """ > +A+="a" > +A+ = "b" > ++ = "c" > +""" > + ambigous_assignment = """ > ++= "d" > +""" > + def test_parse_exports(self): > + f = self.parsehelper(self.special_character_assignment) > + d = bb.parse.handle(f.name, self.d)[''] > + self.assertEqual(d.getVar("A"), " a") > + self.assertEqual(d.getVar("A+"), "b") > + self.assertEqual(d.getVar("+"), "c") > + > + f = self.parsehelper(self.ambigous_assignment) > + with self.assertRaises(bb.parse.ParseError) as error: > + bb.parse.handle(f.name, self.d) > + self.assertIn("Empty variable name in assignment", str(error.exception)) This looks good and I appreciate having tests added, thanks! The test name needs changing from test_parse_exports though else it will redefinine one of the other tests. Cheers, Richard
diff --git a/lib/bb/parse/parse_py/ConfHandler.py b/lib/bb/parse/parse_py/ConfHandler.py index 24f81f7e9..1bde59725 100644 --- a/lib/bb/parse/parse_py/ConfHandler.py +++ b/lib/bb/parse/parse_py/ConfHandler.py @@ -20,7 +20,7 @@ from bb.parse import ParseError, resolve_file, ast, logger, handle __config_regexp__ = re.compile( r""" ^ (?P<exp>export\s+)? - (?P<var>[a-zA-Z0-9\-_+.${}/~:]+?) + (?P<var>[a-zA-Z0-9\-_+.${}/~:]*?) (\[(?P<flag>[a-zA-Z0-9\-_+.][a-zA-Z0-9\-_+.@/]*)\])? \s* ( @@ -166,6 +166,8 @@ def feeder(lineno, s, fn, statements, baseconfig=False, conffile=True): m = __config_regexp__.match(s) if m: groupd = m.groupdict() + if groupd['var'] == "": + raise ParseError("Empty variable name in assignment: '%s'" % s, fn, lineno); ast.handleData(statements, fn, lineno, groupd) return diff --git a/lib/bb/tests/parse.py b/lib/bb/tests/parse.py index cb60af364..7598d5904 100644 --- a/lib/bb/tests/parse.py +++ b/lib/bb/tests/parse.py @@ -443,3 +443,23 @@ include \\ in_file.write("\n".join(lines)) in_file.flush() bb.parse.handle(recipename_closed, bb.data.createCopy(self.d)) + + special_character_assignment = """ +A+="a" +A+ = "b" ++ = "c" +""" + ambigous_assignment = """ ++= "d" +""" + def test_parse_exports(self): + f = self.parsehelper(self.special_character_assignment) + d = bb.parse.handle(f.name, self.d)[''] + self.assertEqual(d.getVar("A"), " a") + self.assertEqual(d.getVar("A+"), "b") + self.assertEqual(d.getVar("+"), "c") + + f = self.parsehelper(self.ambigous_assignment) + with self.assertRaises(bb.parse.ParseError) as error: + bb.parse.handle(f.name, self.d) + self.assertIn("Empty variable name in assignment", str(error.exception))
Old code that parse variable names in assignment commands behave differently for variables that ends with special symbol for single-character variable names and multi-character variable names. For example: A+="1" # Change variable ${A}, '+' glued to '=' A+ = "1" # Change variable ${A+} +="1" # Change variable ${+}, the '+' symbol not part of assignment operator + = "1" # Change variable ${+} New code would always assume that '.=', '+=', and ':=' is assignment operator. As result code like the following would raise parsing error +="value" While code with extra spaces would work as before + = "value" # Change variable ${+} This change allow to catch issues in code that generate bitbake configuration files in a manner like "echo ${VARNAME}+=${VALUE} >> conf/local.conf" Signed-off-by: Nikolai Merinov <n.merinov@inango-systems.com> --- lib/bb/parse/parse_py/ConfHandler.py | 4 +++- lib/bb/tests/parse.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-)