BUG#34260344: Disallow empty strings in collection fields

nmariz · nmariz · commit 099e3f1db3d0 · 2022-06-08T21:14:46.000+01:00
Connector/Python allows empty strings ("") in the collection fields
used in methods chained with Collection.modify().

With this patch, this behavior is not allowed. For replacing the entire
document the "$" character should be used instead.

Change-Id: I519a6b0ec41b6f344aaf4bab5e5a89d5475954d9
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -17,6 +17,7 @@ v8.0.30
 - WL#15035: Enforce PEP 7 and PEP 8 coding style
 - WL#14822: Refactor the authentication plugin mechanism
 - WL#14815: Support OpenSSL 3.0
+- BUG#34260344: Disallow empty strings in collection fields
 - BUG#34231226: Generated classes do not work with the latest Protobuf
 - BUG#34228442: Fix NO_BACKSLASH_ESCAPES SQL mode support in c-ext
 - BUG#34223015: Invalidate the usage of non-compatible cursor types
diff --git a/lib/mysqlx/expr.py b/lib/mysqlx/expr.py
@@ -776,6 +776,10 @@ def docpath_array_loc(self):
         )
 
     def document_field(self):
+        if not self.tokens:
+            raise ValueError("Empty string cannot be used as document field")
+        if self.cur_token_type_is(TokenType.DOLLAR):
+            self.consume_token(TokenType.DOLLAR)
         col_id = Message("Mysqlx.Expr.ColumnIdentifier")
         if self.cur_token_type_is(TokenType.IDENT):
             doc_path_item = Message("Mysqlx.Expr.DocumentPathItem")
diff --git a/lib/mysqlx/statement.py b/lib/mysqlx/statement.py
@@ -718,17 +718,20 @@ class UpdateSpec:
         update_type (int): The update type.
         source (str): The source.
         value (Optional[str]): The value.
+
+    Raises:
+        ProgrammingError: If `source` is invalid.
     """
 
     def __init__(self, update_type, source, value=None):
         if update_type == mysqlxpb_enum("Mysqlx.Crud.UpdateOperation.UpdateType.SET"):
             self._table_set(source, value)
         else:
             self.update_type = update_type
-            self.source = source
-            if len(source) > 0 and source[0] == "$":
-                self.source = source[1:]
-            self.source = ExprParser(self.source, False).document_field().identifier
+            try:
+                self.source = ExprParser(source, False).document_field().identifier
+            except ValueError as err:
+                raise ProgrammingError(f"{err}")
             self.value = value
 
     def _table_set(self, source, value):
@@ -897,7 +900,7 @@ def patch(self, doc):
             )
         self._update_ops["patch"] = UpdateSpec(
             mysqlxpb_enum("Mysqlx.Crud.UpdateOperation.UpdateType.MERGE_PATCH"),
-            "",
+            "$",
             doc.expr() if isinstance(doc, ExprParser) else doc,
         )
         self._changed = True
diff --git a/tests/test_mysqlx_crud.py b/tests/test_mysqlx_crud.py
@@ -1889,6 +1889,38 @@ def test_modify(self):
         result = collection.modify("").unset(["young"])
         self.assertRaises(mysqlx.ProgrammingError, result.execute)
 
+        # test empty strings in collection fields
+        for string in ("", " ", "  "):
+            self.assertRaises(
+                mysqlx.ProgrammingError,
+                collection.modify("$._id == 1").set,
+                string,
+                {"name": "Maria", "age": 78},
+            )
+            self.assertRaises(
+                mysqlx.ProgrammingError,
+                collection.modify("$._id == 1").unset,
+                string,
+            )
+            self.assertRaises(
+                mysqlx.ProgrammingError,
+                collection.modify("$._id == 1").change,
+                string,
+                {"name": "Maria", "age": 78},
+            )
+            self.assertRaises(
+                mysqlx.ProgrammingError,
+                collection.modify("$._id == 1").array_insert,
+                string,
+                [2, 3, 4],
+            )
+            self.assertRaises(
+                mysqlx.ProgrammingError,
+                collection.modify("$._id == 1").array_append,
+                string,
+                [2, 3, 4],
+            )
+
         self.schema.drop_collection(collection_name)
 
     @unittest.skipIf(tests.MYSQL_VERSION < (8, 0, 4), "Unavailable")
