Re: [PATCH v2] parse: Forbid ambiguous assignments to ${.}, ${+}, and ${:} variables

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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