[TASK] Doctrine: Create named parameters for value setting operations 62/47762/3
authorMorton Jonuschat <m.jonuschat@mojocode.de>
Mon, 18 Apr 2016 19:13:03 +0000 (21:13 +0200)
committerChristian Kuhn <lolli@schwarzbu.ch>
Tue, 19 Apr 2016 11:52:14 +0000 (13:52 +0200)
To reduce the risk of SQL injections methods used to set values in the
database have been modified to create named parameters by default.

To work with SQL fragments/expressions this behavior can be disabled by
setting $createNamedParameter to false.

Releases: master
Resolves: #75755
Change-Id: I03bff29b0d50c0a3e7d7dbf27538f1c3dfca51da
Reviewed-on: https://review.typo3.org/47762
Reviewed-by: Susanne Moog <typo3@susannemoog.de>
Tested-by: Susanne Moog <typo3@susannemoog.de>
Reviewed-by: Mathias Schreiber <mathias.schreiber@wmdb.de>
Tested-by: Mathias Schreiber <mathias.schreiber@wmdb.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
typo3/sysext/core/Classes/Database/Query/QueryBuilder.php
typo3/sysext/core/Tests/Unit/Database/Query/QueryBuilderTest.php

index 6189ce1..c6f6218 100644 (file)
@@ -562,14 +562,15 @@ class QueryBuilder
      *
      * @param string $key The column to set.
      * @param string $value The value, expression, placeholder, etc.
+     * @param bool $createNamedParameter Automatically create a named parameter for the value
      *
      * @return QueryBuilder This QueryBuilder instance.
      */
-    public function set(string $key, $value): QueryBuilder
+    public function set(string $key, $value, bool $createNamedParameter = true): QueryBuilder
     {
         $this->concreteQueryBuilder->set(
             $this->quoteIdentifier($key),
-            $value
+            $createNamedParameter ? $this->createNamedParameter($value) : $value
         );
 
         return $this;
@@ -645,7 +646,7 @@ class QueryBuilder
      *
      * @return QueryBuilder This QueryBuilder instance.
      */
-    public function addGroupBy(...$groupBy)
+    public function addGroupBy(...$groupBy): QueryBuilder
     {
         $this->concreteQueryBuilder->addGroupBy(...$this->quoteIdentifiers($groupBy));
 
@@ -657,14 +658,16 @@ class QueryBuilder
      *
      * @param string $column The column into which the value should be inserted.
      * @param string $value The value that should be inserted into the column.
+     * @param bool $createNamedParameter Automatically create a named parameter for the value
      *
      * @return QueryBuilder This QueryBuilder instance.
      */
-    public function setValue(string $column, $value)
+    public function setValue(string $column, $value, bool $createNamedParameter = true): QueryBuilder
     {
+
         $this->concreteQueryBuilder->setValue(
             $this->quoteIdentifier($column),
-            $value
+            $createNamedParameter ? $this->createNamedParameter($value) : $value
         );
 
         return $this;
@@ -675,11 +678,18 @@ class QueryBuilder
      * Replaces any previous values, if any.
      *
      * @param array $values The values to specify for the insert query indexed by column names.
+     * @param bool $createNamedParameters Automatically create named parameters for all values
      *
      * @return QueryBuilder This QueryBuilder instance.
      */
-    public function values(array $values)
+    public function values(array $values, bool $createNamedParameters = true): QueryBuilder
     {
+        if ($createNamedParameters === true) {
+            foreach ($values as &$value) {
+                $value = $this->createNamedParameter($value);
+            }
+        }
+
         $this->concreteQueryBuilder->values($this->quoteColumnValuePairs($values));
 
         return $this;
@@ -856,7 +866,7 @@ class QueryBuilder
      *
      * @return string
      */
-    public function createPositionalParameter($value, int $type = \PDO::PARAM_STR)
+    public function createPositionalParameter($value, int $type = \PDO::PARAM_STR): string
     {
         return $this->concreteQueryBuilder->createPositionalParameter($value, $type);
     }
index 80c8db3..35a547d 100644 (file)
@@ -486,7 +486,10 @@ class QueryBuilderTest extends UnitTestCase
         $this->connection->quoteIdentifier('aField')
             ->shouldBeCalled()
             ->willReturnArgument(0);
-        $this->concreteQueryBuilder->set('aField', 'aValue')
+        $this->concreteQueryBuilder->createNamedParameter('aValue', Argument::cetera())
+            ->shouldBeCalled()
+            ->willReturn(':dcValue1');
+        $this->concreteQueryBuilder->set('aField', ':dcValue1')
             ->shouldBeCalled()
             ->willReturn($this->subject);
 
@@ -496,6 +499,22 @@ class QueryBuilderTest extends UnitTestCase
     /**
      * @test
      */
+    public function setWithoutNamedParameterQuotesIdentifierAndDelegatesToConcreteQueryBuilder()
+    {
+        $this->connection->quoteIdentifier('aField')
+            ->shouldBeCalled()
+            ->willReturnArgument(0);
+        $this->concreteQueryBuilder->createNamedParameter(Argument::cetera())->shouldNotBeCalled();
+        $this->concreteQueryBuilder->set('aField', 'aValue')
+            ->shouldBeCalled()
+            ->willReturn($this->subject);
+
+        $this->subject->set('aField', 'aValue', false);
+    }
+
+    /**
+     * @test
+     */
     public function whereDelegatesToConcreteQueryBuilder()
     {
         $this->concreteQueryBuilder->where('uid=1', 'type=9')
@@ -567,7 +586,10 @@ class QueryBuilderTest extends UnitTestCase
         $this->connection->quoteIdentifier('aField')
             ->shouldBeCalled()
             ->willReturnArgument(0);
-        $this->concreteQueryBuilder->setValue('aField', 'aValue')
+        $this->concreteQueryBuilder->createNamedParameter('aValue', Argument::cetera())
+            ->shouldBeCalled()
+            ->willReturn(':dcValue1');
+        $this->concreteQueryBuilder->setValue('aField', ':dcValue1')
             ->shouldBeCalled()
             ->willReturn($this->subject);
 
@@ -577,8 +599,44 @@ class QueryBuilderTest extends UnitTestCase
     /**
      * @test
      */
+    public function setValueWithoudNamedParameterQuotesIdentifierAndDelegatesToConcreteQueryBuilder()
+    {
+        $this->connection->quoteIdentifier('aField')
+            ->shouldBeCalled()
+            ->willReturnArgument(0);
+        $this->concreteQueryBuilder->setValue('aField', 'aValue')
+            ->shouldBeCalled()
+            ->willReturn($this->subject);
+
+        $this->subject->setValue('aField', 'aValue', false);
+    }
+
+    /**
+     * @test
+     */
     public function valuesQuotesIdentifiersAndDelegatesToConcreteQueryBuilder()
     {
+        $this->connection->quoteColumnValuePairs(['aField' => ':dcValue1', 'aValue' => ':dcValue2'])
+            ->shouldBeCalled()
+            ->willReturnArgument(0);
+        $this->concreteQueryBuilder->createNamedParameter(1, Argument::cetera())
+            ->shouldBeCalled()
+            ->willReturn(':dcValue1');
+        $this->concreteQueryBuilder->createNamedParameter(2, Argument::cetera())
+            ->shouldBeCalled()
+            ->willReturn(':dcValue2');
+        $this->concreteQueryBuilder->values(['aField' => ':dcValue1', 'aValue' => ':dcValue2'])
+            ->shouldBeCalled()
+            ->willReturn($this->subject);
+
+        $this->subject->values(['aField' => 1, 'aValue' => 2]);
+    }
+
+    /**
+     * @test
+     */
+    public function valuesWithoutNamedParametersQuotesIdentifiersAndDelegatesToConcreteQueryBuilder()
+    {
         $this->connection->quoteColumnValuePairs(['aField' => 1, 'aValue' => 2])
             ->shouldBeCalled()
             ->willReturnArgument(0);
@@ -586,7 +644,7 @@ class QueryBuilderTest extends UnitTestCase
             ->shouldBeCalled()
             ->willReturn($this->subject);
 
-        $this->subject->values(['aField' => 1, 'aValue' => 2]);
+        $this->subject->values(['aField' => 1, 'aValue' => 2], false);
     }
 
     /**