diff --git a/core-bundle/src/Resources/contao/drivers/DC_Table.php b/core-bundle/src/Resources/contao/drivers/DC_Table.php index 75fa0821f52..ffb2bf97a89 100644 --- a/core-bundle/src/Resources/contao/drivers/DC_Table.php +++ b/core-bundle/src/Resources/contao/drivers/DC_Table.php @@ -3837,7 +3837,10 @@ public function ajaxTreeView($id, $level) protected function generateTree($table, $id, $arrPrevNext, $blnHasSorting, $intMargin=0, $arrClipboard=null, $blnCircularReference=false, $protectedPage=false, $blnNoRecursion=false, $arrFound=array()) { // Must be either visible in the root trail or allowed by permissions (or their children) - if (!\in_array($id, $this->visibleRootTrails) && !\in_array($id, $this->root) && !\in_array($id, $this->rootChildren)) + // In the extended tree view (mode 6) we have to check on the parent table only + $needsCheck = ($GLOBALS['TL_DCA'][$this->strTable]['list']['sorting']['mode'] ?? null) == 5 || $table !== $this->strTable; + + if ($needsCheck && !\in_array($id, $this->visibleRootTrails) && !\in_array($id, $this->root) && !\in_array($id, $this->rootChildren)) { return ''; } diff --git a/core-bundle/src/Resources/contao/library/Contao/Database/Statement.php b/core-bundle/src/Resources/contao/library/Contao/Database/Statement.php index b2f7e065a8e..47ba50db1ca 100644 --- a/core-bundle/src/Resources/contao/library/Contao/Database/Statement.php +++ b/core-bundle/src/Resources/contao/library/Contao/Database/Statement.php @@ -12,7 +12,8 @@ use Contao\Database; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\Driver\ResultStatement; +use Doctrine\DBAL\Driver\Result as DoctrineResult; +use Doctrine\DBAL\Exception\DriverException; /** * Create and execute queries @@ -44,7 +45,7 @@ class Statement /** * Database statement - * @var ResultStatement + * @var DoctrineResult */ protected $statement; @@ -55,27 +56,23 @@ class Statement protected $strQuery; /** - * Autocommit indicator - * @var boolean + * @var array */ - protected $blnDisableAutocommit = false; + private $arrSetParams = array(); /** - * Result cache * @var array */ - protected static $arrCache = array(); + private $arrLastUsedParams = array(); /** * Validate the connection resource and store the query string * - * @param Connection $resConnection The connection resource - * @param boolean $blnDisableAutocommit Optionally disable autocommitting + * @param Connection $resConnection The connection resource */ - public function __construct(Connection $resConnection, $blnDisableAutocommit=false) + public function __construct(Connection $resConnection) { $this->resConnection = $resConnection; - $this->blnDisableAutocommit = $blnDisableAutocommit; } /** @@ -124,27 +121,7 @@ public function prepare($strQuery) } $this->strQuery = trim($strQuery); - - // Auto-generate the SET/VALUES subpart - if (strncasecmp($this->strQuery, 'INSERT', 6) === 0 || strncasecmp($this->strQuery, 'UPDATE', 6) === 0) - { - $this->strQuery = str_replace('%s', '%p', $this->strQuery); - } - - // Replace wildcards - $arrChunks = preg_split("/('[^']*')/", $this->strQuery, -1, PREG_SPLIT_DELIM_CAPTURE|PREG_SPLIT_NO_EMPTY); - - foreach ($arrChunks as $k=>$v) - { - if (0 === strncmp($v, "'", 1)) - { - continue; - } - - $arrChunks[$k] = str_replace('?', '%s', $v); - } - - $this->strQuery = implode('', $arrChunks); + $this->arrLastUsedParams = array(); return $this; } @@ -166,33 +143,50 @@ public function prepare($strQuery) */ public function set($arrParams) { - $strQuery = ''; - $arrParams = $this->escapeParams($arrParams); + if (substr_count($this->strQuery, '%s') !== 1 || !\in_array(strtoupper(substr($this->strQuery, 0, 6)), array('INSERT', 'UPDATE'), true)) + { + trigger_deprecation('contao/core-bundle', '4.13', 'Using "%s()" is only supported for INSERT and UPDATE queries with the "%%s" placeholder. This will throw an exception in Contao 5.0.', __METHOD__); + + return $this; + } + + $this->arrSetParams = array_values($arrParams); + + $arrParamNames = array_map( + static function ($strName) + { + if (!preg_match('/^[A-Za-z0-9_$]+$/', $strName)) + { + throw new \RuntimeException(sprintf('Invalid column name "%s" in %s()', $strName, __METHOD__)); + } + + return Database::quoteIdentifier($strName); + }, + array_keys($arrParams) + ); // INSERT if (strncasecmp($this->strQuery, 'INSERT', 6) === 0) { $strQuery = sprintf( '(%s) VALUES (%s)', - implode(', ', array_map('Database::quoteIdentifier', array_keys($arrParams))), - str_replace('%', '%%', implode(', ', $arrParams)) + implode(', ', $arrParamNames), + implode(', ', array_fill(0, \count($arrParams), '?')) ); } // UPDATE - elseif (strncasecmp($this->strQuery, 'UPDATE', 6) === 0) + else { - $arrSet = array(); - - foreach ($arrParams as $k=>$v) + if (!$arrParamNames) { - $arrSet[] = Database::quoteIdentifier($k) . '=' . $v; + throw new \InvalidArgumentException('Set array must not be empty for UPDATE queries'); } - $strQuery = 'SET ' . str_replace('%', '%%', implode(', ', $arrSet)); + $strQuery = 'SET ' . implode('=?, ', $arrParamNames) . '=?'; } - $this->strQuery = str_replace('%p', $strQuery, $this->strQuery); + $this->strQuery = str_replace('%s', $strQuery, $this->strQuery); return $this; } @@ -238,14 +232,14 @@ public function execute() { $arrParams = \func_get_args(); - if (!empty($arrParams) && \is_array($arrParams[0])) + if (\count($arrParams) === 1 && \is_array($arrParams[0])) { + trigger_deprecation('contao/core-bundle', '4.13', 'Using "%s()" with an array parameter has been deprecated and will no longer work in Contao 5.0. Use argument unpacking via ... instead."', __METHOD__); + $arrParams = array_values($arrParams[0]); } - $this->replaceWildcards($arrParams); - - return $this->query(); + return $this->query('', array_merge($this->arrSetParams, $arrParams)); } /** @@ -257,7 +251,7 @@ public function execute() * * @throws \Exception If the query string is empty */ - public function query($strQuery='') + public function query($strQuery='', array $arrParams = array()) { if (!empty($strQuery)) { @@ -270,8 +264,39 @@ public function query($strQuery='') throw new \Exception('Empty query string'); } + $arrParams = array_map( + static function ($varParam) + { + if (\is_string($varParam) || \is_bool($varParam) || \is_float($varParam) || \is_int($varParam) || $varParam === null) + { + return $varParam; + } + + return serialize($varParam); + }, + $arrParams + ); + + $this->arrLastUsedParams = $arrParams; + // Execute the query - $this->statement = $this->resConnection->executeQuery($this->strQuery); + try + { + $this->statement = $this->resConnection->executeQuery($this->strQuery, $arrParams); + } + catch (DriverException $exception) + { + if ($arrParams !== array(null)) + { + throw $exception; + } + + // Backwards compatibility for calling Statement::execute(null) + $this->statement = $this->resConnection->executeQuery($this->strQuery); + + // Only trigger the deprecation if the parameter count was the reason for the exception and the previous call did not throw + trigger_deprecation('contao/core-bundle', '4.13', 'Using "%s::execute(null)" has been deprecated and will no longer work in Contao 5.0. Use "%s::execute() instead."', __CLASS__, __CLASS__); + } // No result set available if ($this->statement->columnCount() < 1) @@ -289,9 +314,13 @@ public function query($strQuery='') * @param array $arrValues The values array * * @throws \Exception If $arrValues has too few values to replace the wildcards in the query string + * + * @deprecated Deprecated since Contao 4.13, to be removed in Contao 5.0. */ protected function replaceWildcards($arrValues) { + trigger_deprecation('contao/core-bundle', '4.13', 'Using "%s()" has been deprecated and will no longer work in Contao 5.0.', __METHOD__); + $arrValues = $this->escapeParams($arrValues); $this->strQuery = preg_replace('/(?strQuery); @@ -308,9 +337,13 @@ protected function replaceWildcards($arrValues) * @param array $arrValues The values array * * @return array The array with the escaped values + * + * @deprecated Deprecated since Contao 4.13, to be removed in Contao 5.0. */ protected function escapeParams($arrValues) { + trigger_deprecation('contao/core-bundle', '4.13', 'Using "%s()" has been deprecated and will no longer work in Contao 5.0.', __METHOD__); + foreach ($arrValues as $k=>$v) { switch (\gettype($v)) @@ -341,10 +374,14 @@ protected function escapeParams($arrValues) * Explain the current query * * @return string The explanation string + * + * @deprecated Deprecated since Contao 4.13, to be removed in Contao 5.0. */ public function explain() { - return $this->resConnection->fetchAssociative('EXPLAIN ' . $this->strQuery); + trigger_deprecation('contao/core-bundle', '4.13', 'Using "%s()" has been deprecated and will no longer work in Contao 5.0.', __METHOD__); + + return $this->resConnection->fetchAssociative('EXPLAIN ' . $this->strQuery, $this->arrLastUsedParams); } /** diff --git a/core-bundle/src/Resources/contao/library/Contao/Model.php b/core-bundle/src/Resources/contao/library/Contao/Model.php index cace7e9da55..a929b9db5b1 100644 --- a/core-bundle/src/Resources/contao/library/Contao/Model.php +++ b/core-bundle/src/Resources/contao/library/Contao/Model.php @@ -1072,7 +1072,7 @@ protected static function find(array $arrOptions) } $objStatement = static::preFind($objStatement); - $objResult = $objStatement->execute($arrOptions['value'] ?? null); + $objResult = $objStatement->execute(...(array) ($arrOptions['value'] ?? array())); if ($objResult->numRows < 1) { @@ -1155,7 +1155,7 @@ public static function countBy($strColumn=null, $varValue=null, array $arrOption $strQuery = static::buildCountQuery($arrOptions); - return (int) Database::getInstance()->prepare($strQuery)->execute($arrOptions['value'])->count; + return (int) Database::getInstance()->prepare($strQuery)->execute(...(array) ($arrOptions['value'] ?? array()))->count; } /** diff --git a/core-bundle/src/Search/Indexer/DefaultIndexer.php b/core-bundle/src/Search/Indexer/DefaultIndexer.php index 8507e3cf547..6d0845f3cfd 100644 --- a/core-bundle/src/Search/Indexer/DefaultIndexer.php +++ b/core-bundle/src/Search/Indexer/DefaultIndexer.php @@ -43,6 +43,10 @@ public function index(Document $document): void $this->throwBecause('Cannot index empty response.'); } + if (($canonical = $document->extractCanonicalUri()) && ((string) $canonical !== (string) $document->getUri())) { + $this->throwBecause(sprintf('Ignored because canonical URI "%s" does not match document URI.', $canonical)); + } + try { $title = $document->getContentCrawler()->filterXPath('//head/title')->first()->text(null, true); } catch (\Exception $e) { diff --git a/core-bundle/tests/Contao/Database/StatementTest.php b/core-bundle/tests/Contao/Database/StatementTest.php new file mode 100644 index 00000000000..6ccfbbe824f --- /dev/null +++ b/core-bundle/tests/Contao/Database/StatementTest.php @@ -0,0 +1,274 @@ +createMock(Connection::class)); + + if ($query) { + $statement->prepare($query); + } + + $this->expectDeprecation('%sUsing "%s()" is only supported for INSERT and UPDATE queries with the "%%s" placeholder%s'); + + $this->assertSame($statement, $statement->set(['foo' => 'bar'])); + } + + public function getDeprecatedSetQueries(): \Generator + { + yield ['']; + yield ['SELECT * FROM %s']; + yield ["SELECT 'INSERT' FROM %s"]; + yield ['INSERT INTO missing_placeholder']; + yield ['UPDATE missing_placeholder']; + yield ['INSERT INTO two_placeholders %s %s']; + yield ['UPDATE two_placeholders %s %s']; + } + + /** + * @group legacy + */ + public function testExplainQuery(): void + { + $doctrineResult = $this->createMock(Result::class); + $doctrineResult + ->method('columnCount') + ->willReturn(1) + ; + + $connection = $this->createMock(Connection::class); + $connection + ->method('executeQuery') + ->willReturn($doctrineResult) + ; + + $connection + ->method('quoteIdentifier') + ->willReturnArgument(0) + ; + + $connection + ->method('getDatabasePlatform') + ->willReturn($this->createMock(AbstractPlatform::class)) + ; + + $container = new Container(); + $container->set('database_connection', $connection); + + System::setContainer($container); + + $connection + ->method('fetchAssociative') + ->withConsecutive( + ['EXPLAIN SELECT * FROM tl_content', []], + ['EXPLAIN SELECT * FROM tl_content WHERE id = ?', [123]], + ['EXPLAIN INSERT INTO tl_content (foo) VALUES (?)', ['bar']], + ['EXPLAIN UPDATE tl_content SET foo=? WHERE id = ?', ['bar', 123]] + ) + ->willReturn([]) + ; + + $this->expectDeprecation('%sUsing "%s::explain()" has been deprecated%s'); + + $statement = (new Statement($connection)); + $statement->query('SELECT * FROM tl_content'); + $statement->explain(); + + $statement = (new Statement($connection)); + $statement->prepare('SELECT * FROM tl_content WHERE id = ?')->execute([123]); + $statement->explain(); + + $statement = (new Statement($connection)); + $statement->prepare('INSERT INTO tl_content %s')->set(['foo' => 'bar'])->execute(); + $statement->explain(); + + $statement = (new Statement($connection)); + $statement->prepare('UPDATE tl_content %s WHERE id = ?')->set(['foo' => 'bar'])->execute([123]); + $statement->explain(); + } + + /** + * @group legacy + * @dataProvider getQueriesWithParametersAndSets + */ + public function testBackwardsCompatibleReplacesParametersAndSets(string $query, string $expected, array $params = null, array $set = null): void + { + $doctrineResult = $this->createMock(Result::class); + $doctrineResult + ->method('columnCount') + ->willReturn(1) + ; + + $connection = $this->createMock(Connection::class); + $connection + ->expects($this->exactly(2)) + ->method('executeQuery') + ->willReturnCallback( + function (string $query, array $params) use ($doctrineResult, $expected) { + $params = array_map( + static function ($param) { + if (\is_bool($param)) { + return (int) $param; + } + + if (\is_object($param) || \is_array($param)) { + $param = serialize($param); + } + + if (\is_string($param)) { + return "'".str_replace("'", "''", $param)."'"; + } + + return $param ?? 'NULL'; + }, + $params + ); + + $builtQuery = ''; + + foreach (explode('?', $query) as $index => $queryPart) { + $builtQuery .= $params[$index - 1] ?? ''; + $builtQuery .= $queryPart; + } + + $this->assertSame($expected, $builtQuery); + + return $doctrineResult; + } + ) + ; + + $connection + ->method('quoteIdentifier') + ->willReturnArgument(0) + ; + + $connection + ->method('getDatabasePlatform') + ->willReturn($this->createMock(AbstractPlatform::class)) + ; + + $container = new Container(); + $container->set('database_connection', $connection); + + System::setContainer($container); + + $statement = (new Statement($connection)); + $statement->prepare($query); + + if ($set) { + $statement->set($set); + } + + $statement->execute(...($params ?? [])); + + $statement = (new Statement($connection)); + $statement->prepare($query); + + if ($set) { + $statement->set($set); + } + + $this->expectDeprecation('%sUsing "%s" with an array parameter has been deprecated%s'); + + $statement->execute($params ?? []); + } + + public function getQueriesWithParametersAndSets(): \Generator + { + yield [ + 'SELECT id FROM tl_content', + 'SELECT id FROM tl_content', + ]; + + yield [ + 'SELECT id FROM tl_content WHERE boolCol = ? AND intCol = ? AND floatCol = ? AND stringCol = ? AND nullCol = ?', + "SELECT id FROM tl_content WHERE boolCol = 0 AND intCol = 0 AND floatCol = 0 AND stringCol = '' AND nullCol = NULL", + [false, 0, 0.0, '', null], + ]; + + yield [ + 'SELECT id FROM tl_content WHERE boolCol = ? AND intCol = ? AND floatCol = ? AND stringCol = ?', + "SELECT id FROM tl_content WHERE boolCol = 1 AND intCol = 123456 AND floatCol = 123.456 AND stringCol = 'foo''bar'", + [true, 123456, 123.456, 'foo\'bar'], + ]; + + yield [ + 'SELECT id FROM tl_content WHERE objectCol = ? AND arrayCol = ?', + "SELECT id FROM tl_content WHERE objectCol = 'O:8:\"stdClass\":0:{}' AND arrayCol = 'a:0:{}'", + [new \stdClass(), []], + ]; + + yield [ + 'SELECT id FROM tl_content WHERE objectCol = ? AND arrayCol = ?', + "SELECT id FROM tl_content WHERE objectCol = 'O:8:\"stdClass\":2:{s:1:\"a\";i:1;s:1:\"b\";i:1;}' AND arrayCol = 'a:2:{i:0;i:1;i:1;i:2;}'", + [(object) ['a' => 1, 'b' => 1], [1, 2]], + ]; + + yield [ + 'SELECT id FROM tl_content WHERE arrayCol = ? AND objectCol = ?', + "SELECT id FROM tl_content WHERE arrayCol = 'a:2:{i:0;i:1;i:1;i:2;}' AND objectCol = 'O:8:\"stdClass\":2:{s:1:\"a\";i:1;s:1:\"b\";i:1;}'", + [[1, 2], (object) ['a' => 1, 'b' => 1]], + ]; + + yield [ + 'INSERT INTO tl_content %s', + "INSERT INTO tl_content (boolCol, intCol, floatCol, stringCol, nullCol) VALUES (1, 123456, 123.456, 'foo''bar', NULL)", + null, + [ + 'boolCol' => true, + 'intCol' => 123456, + 'floatCol' => 123.456, + 'stringCol' => 'foo\'bar', + 'nullCol' => null, + ], + ]; + + yield [ + 'UPDATE tl_content %s WHERE id = ?', + "UPDATE tl_content SET boolCol=1, intCol=123456, floatCol=123.456, stringCol='foo''bar', nullCol=NULL WHERE id = 123", + [123], + [ + 'boolCol' => true, + 'intCol' => 123456, + 'floatCol' => 123.456, + 'stringCol' => 'foo\'bar', + 'nullCol' => null, + ], + ]; + + yield [ + "SELECT id FROM tl_content WHERE headline = '%%%-special' and type = ?", + "SELECT id FROM tl_content WHERE headline = '%%%-special' and type = ' BOBBY TABLES -- a'", + [' BOBBY TABLES -- a', 'b'], + ]; + } +} diff --git a/core-bundle/tests/Search/Indexer/DefaultIndexerTest.php b/core-bundle/tests/Search/Indexer/DefaultIndexerTest.php index c0c28dedd3d..3b0aa3a969d 100644 --- a/core-bundle/tests/Search/Indexer/DefaultIndexerTest.php +++ b/core-bundle/tests/Search/Indexer/DefaultIndexerTest.php @@ -71,6 +71,12 @@ public function indexProvider(): \Generator 'Cannot index empty response.', ]; + yield 'Test does not index if rel="canonical" does not match current page' => [ + new Document(new Uri('https://example.com/page'), 200, [], ''), + null, + 'Ignored because canonical URI "https://example.com/other-page" does not match document URI.', + ]; + yield 'Test does not index if page ID could not be determined' => [ new Document(new Uri('https://example.com/no-page-id'), 200, [], ''), null, @@ -177,6 +183,33 @@ public function indexProvider(): \Generator null, true, ]; + + yield 'Test valid index with self-referencing rel="canonical"' => [ + new Document(new Uri('https://example.com/valid'), 200, [], 'HTML page title'), + [ + 'url' => 'https://example.com/valid', + 'content' => 'HTML page title', + 'protected' => '1', + 'groups' => [42], + 'pid' => 2, + 'title' => 'JSON-LD page title', + 'language' => 'de', + 'meta' => [ + [ + '@context' => ['contao' => 'https://schema.contao.org/'], + '@type' => 'https://schema.contao.org/Page', + 'https://schema.contao.org/title' => 'JSON-LD page title', + 'https://schema.contao.org/pageId' => 2, + 'https://schema.contao.org/noSearch' => false, + 'https://schema.contao.org/protected' => true, + 'https://schema.contao.org/groups' => [42], + 'https://schema.contao.org/fePreview' => false, + ], + ], + ], + null, + true, + ]; } /**