[TYPO3-core] RFC #12512: Cannot move multiple records with clipboard

[Rupert Germann]

Hi Xavier,

many thanks for your detailed bug description.
The patch looks quite obvious. I'll try to reproduce it on a Postgres 
installation when I'm back home tomorrow.

greets
rupert

Xavier Perseguers schrieb:
> Hi,
> 
> This is an SVN patch request.
> 
> Type: Bugfix, nobrainer
> 
> Bugtracker reference:
> http://bugs.typo3.org/view.php?id=12512
> 
> Branch: Trunk (at least) but I guess this bug is present in all branches
> 
> Problem:
> When trying to move multiple records (e.g. news categories, but NOT 
> pages) for a sysfolder/page to another one using clipboard #1-#3, the 
> paste operation dies with a parsing error in the where clause.
> 
> Only first record is actually moved, the others stay in the original 
> sysfolder.
> 
> When moving a single record with clipboard #1-#3, the error occurs but 
> record is moved anyway.
> 
> 
> Analyze of the bug:
> Error occurs in t3lib_BEfunc::getRecordsByField() as second argument 
> $theField is *empty*, this variable is used to create a query:
> 
> $res = $GLOBALS['TYPO3_DB']->exec_SELECTquery(
>     '*',
>     $theTable,
>     $theField.'='.$GLOBALS['TYPO3_DB']->fullQuoteStr($theValue, $theTable).
>         ($useDeleteClause ? t3lib_BEfunc::deleteClause($theTable).' ' : 
> '').
>         t3lib_BEfunc::versioningPlaceholderClause($theTable).' '.
>         $whereClause,    // whereClauseMightContainGroupOrderBy
>     $groupBy,
>     $orderBy,
>     $limit
> );
> 
> It's obvious that if $theField is empty, condition will start with a 
> comparison operator (=) but without left hand member and won't ever be 
> valid.
> 
> How is it possible that $theField is (sometimes) empty might you ask? 
> Well, when moving records with the clipboard, method 
> t3lib_tcemain::moveRecord_raw() is called for each record to be moved. 
> At line 3695, after having moved the record, it calls method 
> moveL10nOverlayRecords() to move related translations. This is where the 
> problem relies as seen in the enclosed patch as this latest method calls 
> t3lib_BEfunc::getRecordsByField() with a possible empty value for 
> $theField. Bing!!!
> 
> Why was it never detected?
> As said, the query won't ever be valid if $theField is empty. But nobody 
> ever saw the problem because the error is silently forgotten by a HTTP 
> redirect or something of that kind when using MySQL. However, when using 
> DBAL (and *not* using 'native' driver!), each and every query is 
> properly parsed before being sent to the corresponding DBMS. And... yes! 
> you read it right! DBAL showed me that Core had a problem :-)
> 
> Solution:
> It's obvious: make sure $theField won't be empty before calling 
> t3lib_BEfunc::getRecordsByField().
> 
> I don't know how this patch will be reviewed as it is a bit tricky to 
> reproduce without a DBAL environment that does not run on MySQL. 
> However, even with MySQL, you may add debugging code into 
> t3lib_BEfunc::getRecordsByField, filtering by $theTable and showing 
> that, e.g., when moving tt_news categories (that is, records from table 
> 'tt_news_cat'), the generated query is invalid when you try to move one 
> or more records using clipboard #1-#3. I least, it seems easy for 
> anybody to give a +1 by reading and "thinking" :-) Hopefully, this will 
> be sufficient...
> 
> Cheers.
>