From caab09b59f0e2b8606cf7ce781e9029964f97966 Mon Sep 17 00:00:00 2001 From: Ryunosuke O'Neil Date: Mon, 18 Nov 2024 16:06:00 +0100 Subject: [PATCH 1/2] fix (Transformation): use parameterised query in addTransformation --- src/DIRAC/Core/Utilities/MySQL.py | 6 +- .../DB/TransformationDB.py | 64 +++++++++++-------- 2 files changed, 40 insertions(+), 30 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 6e7682a08fa..f3eb27ad911 100755 --- a/src/DIRAC/TransformationSystem/DB/TransformationDB.py +++ b/src/DIRAC/TransformationSystem/DB/TransformationDB.py @@ -160,34 +160,42 @@ def addTransformation( if not res["OK"]: return S_ERROR("Failed to parse the transformation body") body = res["Value"] - req = ( - "INSERT INTO Transformations (TransformationName,Description,LongDescription, \ - CreationDate,LastUpdate,AuthorDN,AuthorGroup,Type,Plugin,AgentType,\ - FileMask,Status,TransformationGroup,GroupSize,\ - InheritedFrom,Body,MaxNumberOfTasks,EventsPerTask)\ - VALUES ('%s','%s','%s',\ - UTC_TIMESTAMP(),UTC_TIMESTAMP(),'%s','%s','%s','%s','%s',\ - '%s','New','%s',%f,\ - %d,%s,%d,%d);" - % ( - transName, - description, - longDescription, - authorDN, - authorGroup, - transType, - plugin, - agentType, - fileMask, - transformationGroup, - groupSize, - inheritedFrom, - body, - maxTasks, - eventsPerTask, - ) - ) - res = self._update(req, conn=connection) + + params = { + "TransformationName": transName, + "Description": description, + "LongDescription": longDescription, + "CreationDate": "UTC_TIMESTAMP()", + "LastUpdate": "UTC_TIMESTAMP()", + "AuthorDN": authorDN, + "AuthorGroup": authorGroup, + "Type": transType, + "Plugin": plugin, + "AgentType": agentType, + "FileMask": fileMask, + "Status": "New", + "TransformationGroup": transformationGroup, + "GroupSize": groupSize, + "InheritedFrom": inheritedFrom, + "Body": body, + "MaxNumberOfTasks": maxTasks, + "EventsPerTask": eventsPerTask, + } + + # A list of parameters that we do not want to substitute as parameters, but directly + # into the statement + # 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 + unparameterised_columns = [ + "Body", + "CreationDate", + "LastUpdate", + ] + subst = ", ".join(f"%({name})s" if name not in unparameterised_columns else params[name] for name in params) + + 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 From 492b195895b461064c9ba01869509a58b5b06d8d Mon Sep 17 00:00:00 2001 From: Ryunosuke O'Neil Date: Tue, 19 Nov 2024 10:23:05 +0100 Subject: [PATCH 2/2] fix (Transformation): don't use _escapeString for body parameter - use parameterised query instead. --- src/DIRAC/TransformationSystem/DB/TransformationDB.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/DIRAC/TransformationSystem/DB/TransformationDB.py b/src/DIRAC/TransformationSystem/DB/TransformationDB.py index f3eb27ad911..35929d2faab 100755 --- a/src/DIRAC/TransformationSystem/DB/TransformationDB.py +++ b/src/DIRAC/TransformationSystem/DB/TransformationDB.py @@ -156,10 +156,6 @@ def addTransformation( elif res["Message"] != "Transformation does not exist": return res self.lock.acquire() - res = self._escapeString(body) - if not res["OK"]: - return S_ERROR("Failed to parse the transformation body") - body = res["Value"] params = { "TransformationName": transName, @@ -183,11 +179,8 @@ def addTransformation( } # A list of parameters that we do not want to substitute as parameters, but directly - # into the statement - # 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 + # into the statement e.g. functions like "UTC_TIMESTAMP()" unparameterised_columns = [ - "Body", "CreationDate", "LastUpdate", ]