numero2 / contao-proper-filenames

Replaces special characters in filenames right after upload.
https://www.numero2.de/contao/erweiterungen/proper-filenames.html
GNU Lesser General Public License v3.0
13 stars 5 forks source link

Fix error with folder names with special characters #11

Closed fritzmg closed 3 years ago

fritzmg commented 3 years ago

If a folder already has an unsanitized special character (e.g. because the folder existed before this extension was installed and enabled), e.g. a single quote, then the following error will appear when trying to edit that folder:

octrine\DBAL\Exception\SyntaxErrorException:
An exception occurred while executing 'SELECT count(1) AS count
                FROM tl_files
                WHERE type='folder' AND doNotSanitize='1' AND path IN ('files','files/dev','files/dev/test's test')':

SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 's test')' at line 3

  at vendor\doctrine\dbal\lib\Doctrine\DBAL\Driver\AbstractMySQLDriver.php:98
  at Doctrine\DBAL\Driver\AbstractMySQLDriver->convertException('An exception occurred while executing \'SELECT count(1) AS count                FROM tl_files                WHERE type=\'folder\' AND doNotSanitize=\'1\' AND path IN (\'files\',\'files/dev\',\'files/dev/test\'s test\')\':SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near \'s test\')\' at line 3', object(Exception))
     (vendor\sentry\sentry-symfony\src\Tracing\Doctrine\DBAL\TracingDriverForV2.php:109)
  at Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverForV2->convertException('An exception occurred while executing \'SELECT count(1) AS count                FROM tl_files                WHERE type=\'folder\' AND doNotSanitize=\'1\' AND path IN (\'files\',\'files/dev\',\'files/dev/test\'s test\')\':SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near \'s test\')\' at line 3', object(Exception))
     (vendor\doctrine\dbal\lib\Doctrine\DBAL\DBALException.php:182)
  at Doctrine\DBAL\DBALException::wrapException(object(TracingDriverForV2), object(Exception), 'An exception occurred while executing \'SELECT count(1) AS count                FROM tl_files                WHERE type=\'folder\' AND doNotSanitize=\'1\' AND path IN (\'files\',\'files/dev\',\'files/dev/test\'s test\')\':SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near \'s test\')\' at line 3')
     (vendor\doctrine\dbal\lib\Doctrine\DBAL\DBALException.php:159)
  at Doctrine\DBAL\DBALException::driverExceptionDuringQuery(object(TracingDriverForV2), object(Exception), 'SELECT count(1) AS count                FROM tl_files                WHERE type=\'folder\' AND doNotSanitize=\'1\' AND path IN (\'files\',\'files/dev\',\'files/dev/test\'s test\')', array())
     (vendor\doctrine\dbal\lib\Doctrine\DBAL\Connection.php:2225)
  at Doctrine\DBAL\Connection->handleExceptionDuringQuery(object(Exception), 'SELECT count(1) AS count                FROM tl_files                WHERE type=\'folder\' AND doNotSanitize=\'1\' AND path IN (\'files\',\'files/dev\',\'files/dev/test\'s test\')', array(), array())
     (vendor\doctrine\dbal\lib\Doctrine\DBAL\Connection.php:1312)
  at Doctrine\DBAL\Connection->executeQuery('SELECT count(1) AS count                FROM tl_files                WHERE type=\'folder\' AND doNotSanitize=\'1\' AND path IN (\'files\',\'files/dev\',\'files/dev/test\'s test\')')
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Database\Statement.php:274)
  at Contao\Database\Statement->query()
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Database\Statement.php:248)
  at Contao\Database\Statement->execute('folder', '1')
     (vendor\numero2\contao-proper-filenames\src\Resources\contao\library\ProperFilenames\DCAHelper\Files.php:83)
  at numero2\ProperFilenames\DCAHelper\Files::checkParentFolder(null, object(DC_Folder))
     (vendor\contao\core-bundle\src\Resources\contao\drivers\DC_Folder.php:1415)
  at Contao\DC_Folder->edit()
     (vendor\contao\core-bundle\src\Resources\contao\classes\Backend.php:644)
  at Contao\Backend->getBackendModule('files', null)
     (vendor\contao\core-bundle\src\Resources\contao\controllers\BackendMain.php:167)
  at Contao\BackendMain->run()
     (vendor\contao\core-bundle\src\Controller\BackendController.php:48)
  at Contao\CoreBundle\Controller\BackendController->mainAction()
     (vendor\symfony\http-kernel\HttpKernel.php:158)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor\symfony\http-kernel\HttpKernel.php:80)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor\symfony\http-kernel\Kernel.php:201)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (web\index.php:31)
  at require('web\\index.php')
     (web\app.php:4)   

In this case the folder's name was test's test.

This is because the database query to check whether any parent folder has the doNotSanitize option enabled does not currently escape the folder names.

This PR fixes that by using the actual Doctrine\DBAL\Connection instance, together with the Connection::PARAM_STR_ARRAY parameter type, so that a prepared statement with an array parameter can be used.

This PR also fixes the erroneous PaletteManipulator class name (it was missing Contao\ in the beginning).

bennyborn commented 3 years ago

Thanks @fritzmg

Any idea why Contao's database class did no escape the folder name properly?

fritzmg commented 3 years ago

Any idea why Contao's database class did no escape the folder name properly?

You did not use prepared statements - the folder names where imploded as a string and used directly in the query and so there was no possibility to automatically escape anything.

bennyborn commented 3 years ago

You did not use prepared statements - the folder names where imploded as a string and used directly in the query

Well, seems like I was blind 😌 Thanks.