No effect if admin change(update) the parent node

[anonymous]

If we change a parent Node in Admin->Components the old value was shown if we click on save.

In model.py there was an Error?

Your model.py:

query = "UPDATE component_hierarchy SET parent_component='%s' WHERE component='%s'" % (component, parent_component)

the Column parent_component get the value of component and in the WHERE-Clausel the column component looks for content of variable parent_component.

If we change it to:

query = "UPDATE component_hierarchy SET parent_component='%s' WHERE component='%s'" % (parent_component,component)

no Errors any more

[Jun Omae]

The sql queries in that plugin have SQL injection. We should use ​trac:wiki:TracDev/DatabaseApi#Parameterpassing.

[falkb]

In 15747:

fixes #12688: applied patch proposed by anonym which fixed the setting of a new parent (many thanks!)

[Jun Omae]

The SQL injection isn't fixed.

componenthierarchyplugin/trunk/componenthierarchy/model.py

@@ -38 +38 @@
 
     def set_parent_component(self, component, parent_component):
         if parent_component == None or parent_component == "":
-            query = "DELETE FROM component_hierarchy WHERE component='%s'" % component
+            query = "DELETE FROM component_hierarchy WHERE component=%s"
+            args = (component,)
         else:
             if self.has_parent_component(component):
-                query = "UPDATE component_hierarchy SET parent_component='%s' WHERE component='%s'" % (component, parent_component)
+                query = "UPDATE component_hierarchy SET parent_component=%s WHERE component=%s"
+                args = (component, parent_component)
             else:
-                query = "INSERT INTO component_hierarchy (component, parent_component) VALUES ('%s', '%s')" % (component, parent_co
+                query = "INSERT INTO component_hierarchy (component, parent_component) VALUES (%s,%s)"
+                args = (component, parent_component)
 
         if VERSION < '0.12':
             db = self.env.get_db_cnx()
             cursor = db.cursor()
-            cursor.execute(query)
+            cursor.execute(query, args)
             self.__start_transaction(db)
         else:
             @with_transaction(self.env)
             def execute_sql_statement(db):
                 cursor = db.cursor()
-                cursor.execute(query)
+                cursor.execute(query, args)
 
     def rename_component(self, component, new_name):
         query1 = "UPDATE component_hierarchy SET component='%s' WHERE component='%s'" % (new_name, component)

[falkb]

Thanks for your patch, jun66j5! Actually, this was like another ticket for me. Can you confirm if the code works well, I mean, can I blindly commit it? I don't have access to the test bench at present.

[Jun Omae]

In addition, with_transaction is not available with Trac 0.11.x. This plugin doesn't work with Trac 0.11.x due to ImportError.

componenthierarchyplugin/trunk/componenthierarchy/model.py

@@ -6 +6 @@
 from trac import __version__ as VERSION
 from trac.core import *
 from trac.ticket import model
-from trac.db import with_transaction
+try:
+    from trac.db import with_transaction
+except ImportError:
+    with_transaction = None
 
 class ComponentHierarchyModel(Component):
 
     # DB Method
     def __start_transaction(self):
-        if VERSION < '0.12':
+        if with_transaction is None:
             # deprecated in newer versions
             self.db.commit()
             self.db.close()

[Jun Omae]

Replying to falkb:

Thanks for your patch, jun66j5! Actually, this was like another ticket for me. Can you confirm if the code works well, I mean, can I blindly commit it? I don't have access to the test bench at present.

No. SQL Injections exist in other code of this plugin and should be fixed. This patch is just a sample to fix it.

[falkb]

I'll make another ticket as reminder... The problem of #12688 is done.