fix (Transformation): use _escapeString sparingly in favour of parame…

…terised query
DIRACGrid · Nov 18, 2024 · 895af75 · 895af75
1 parent 54e31f6
commit 895af75
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 25 deletions.
diff --git a/src/DIRAC/Core/Utilities/MySQL.py b/src/DIRAC/Core/Utilities/MySQL.py
@@ -752,9 +752,11 @@ def _query(self, cmd, *, conn=None, debug=True):
         return retDict
 
     @captureOptimizerTraces
-    def _update(self, cmd, *, conn=None, debug=True):
+    def _update(self, cmd, *, args=None, conn=None, debug=True):
         """execute MySQL update command
 
+        :param args: parameters passed to cursor.execute(..., args=args) method.
+        :param conn: connection object.
         :param debug: print or not the errors
 
         return S_OK with number of updated registers upon success
@@ -772,7 +774,7 @@ def _update(self, cmd, *, conn=None, debug=True):
 
         try:
             cursor = connection.cursor()
-            res = cursor.execute(cmd)
+            res = cursor.execute(cmd, args=args)
             retDict = S_OK(res)
             if cursor.lastrowid:
                 retDict["lastRowId"] = cursor.lastrowid

diff --git a/src/DIRAC/TransformationSystem/DB/TransformationDB.py b/src/DIRAC/TransformationSystem/DB/TransformationDB.py
@@ -161,20 +161,6 @@ def addTransformation(
             return S_ERROR("Failed to parse the transformation body")
         body = res["Value"]
 
-        escapable_columns = [
-            "TransformationName",
-            "Description",
-            "LongDescription",
-            "AuthorDN",
-            "AuthorGroup",
-            "Type",
-            "Plugin",
-            "AgentType",
-            "FileMask",
-            "Status",
-            "TransformationGroup",
-        ]
-
         params = {
             "TransformationName": transName,
             "Description": description,
@@ -196,16 +182,13 @@ def addTransformation(
             "EventsPerTask": eventsPerTask,
         }
 
-        for key in params:
-            if key in escapable_columns:
-                res = self._escapeString(params[key])
-                if not res["OK"]:
-                    return S_ERROR(f"Could not escape parameter {key}")
-                params[key] = res["Value"]
-        # In an ideal world we would not do this at all, in favour of prepared statements or statement parameters.
-        req = f"INSERT INTO Transformations ({', '.join(params)}) VALUES ({', '.join(params.values())});"
+        # I'm erring on the side of caution by using the _escapeString(body) version of the Body parameter,
+        # but for everything else it seems reasonably safe to use the parameterised query feature
+        subst = ", ".join(f"%({name})s" if name != "Body" else body for name in params)
 
-        res = self._update(req, conn=connection)
+        req = f"INSERT INTO Transformations ({', '.join(params)}) VALUES ({subst});"
+
+        res = self._update(req, args=params, conn=connection)
         if not res["OK"]:
             self.lock.release()
             return res