From 895af752dbd07e94c57bdf278d115781fd5c587b Mon Sep 17 00:00:00 2001 From: Ryunosuke O'Neil Date: Mon, 18 Nov 2024 22:14:39 +0100 Subject: [PATCH] fix (Transformation): use _escapeString sparingly in favour of parameterised query --- src/DIRAC/Core/Utilities/MySQL.py | 6 ++-- .../DB/TransformationDB.py | 29 ++++--------------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/src/DIRAC/Core/Utilities/MySQL.py b/src/DIRAC/Core/Utilities/MySQL.py index 210ff763098..781272822d0 100755 --- 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 index 370715542a2..7e1db34b3f0 100755 --- 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