diff mbox series

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

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

Commit Message

Nikolai Merinov Feb. 3, 2025, 4:42 p.m. UTC
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(-)

Comments

Richard Purdie Feb. 3, 2025, 5:39 p.m. UTC | #1
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 mbox series

Patch

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))