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

Xavier Perseguers typo3 at perseguers.ch
Fri Nov 20 16:36:12 CET 2009


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.

-- 
Xavier Perseguers
http://xavier.perseguers.ch/en

One contribution a day keeps the fork away
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 12512.diff
URL: <http://lists.typo3.org/pipermail/typo3-team-core/attachments/20091120/eb4f1d06/attachment.txt>


More information about the TYPO3-team-core mailing list