Bug #51562

Task #50876: Handling of deleted files in FAL

sys_file records marked as deleted crash file list (AKA: remove sys_file.deleted flag)

Added by Francois Suter over 8 years ago. Updated about 3 years ago.

Status:
Closed
Priority:
Should have
Category:
File Abstraction Layer (FAL)
Target version:
Start date:
2013-08-30
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
6.1
PHP Version:
5.3
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

If some sys_file records have their "deleted" flag set to 1, the File > Filelist module will crash with exception "No file found for given UID". It seems like the deleted flag is not used anymore, but was obviously used at some point in the past, since I have a few records with such a status in my database. I guess I'm not the only one.

Reproducing the error condition is easy: access the sys_file table in some DB management tool, set the deleted flag of some file to 1 and try to access the folder where it is stored in the File module. This results in an exception.

I'm not sure how this is best corrected. I have seen that there's an intention to drop the deleted flag entirely, which seems fine if it is not used anyway, but that would be for 6.2 only. Something must be done for 6.0 and 6.1.


Related issues

Related to TYPO3 Core - Bug #52245: Revert BackendUtility::BEenableFields in FALRejected2013-09-25

Actions
Blocks TYPO3 Core - Bug #51929: Creation of sys_file-table fails due to incorrect keyClosed2013-09-12

Actions
#1

Updated by Steffen Ritter over 8 years ago

all access to the deleted flag in FAL needs to be removed, as well as it's definition in TCA ctrl section and for 6.2 even the sql field itself

This has been defined on the FAL Sprint in Mainz - it just did not happen yet. The missing file handling has been done before.

The patch should be straight forward.

History:
In the original FAL the deleted flag of the table was not part of the concept - the propertiy in the object actually has the same name than the database field which lead to the mistake that it was persisted or peope thought it was the same. By thinking fixing it, people started taking care of the flag which lead to errors and now an undefined state

#2

Updated by Alexander Opitz over 8 years ago

Please can you post a backtrace and which Typo3 6.1 version do you use.

#3

Updated by Alexander Opitz over 8 years ago

@Steffen

Will this change backported to 6.0/6.1? As it may break extensions.

#4

Updated by Francois Suter over 8 years ago

Alexander Opitz wrote:

Please can you post a backtrace and which Typo3 6.1 version do you use.

Version is 6.1.3 (the same happens with 6.1-dev). Here's the backtrace:

Uncaught TYPO3 Exception
#1317178604: No file found for given UID. (More information)

TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException thrown in file
/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/ResourceFactory.php in line 281.

11 TYPO3\CMS\Core\Resource\ResourceFactory::getFileObject(139)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/ProcessedFileRepository.php:
00094:   */
00095:  protected function createDomainObject(array $databaseRow) {
00096:   $originalFile = $this->resourceFactory->getFileObject(intval($databaseRow['original']));
00097:   $originalFile->setStorage($this->resourceFactory->getStorageObject($originalFile->getProperty('storage')));
00098:   $taskType = $databaseRow['task_type'];

10 TYPO3\CMS\Core\Resource\ProcessedFileRepository::createDomainObject(array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/ProcessedFileRepository.php:
00159: 
00160:   if (is_array($databaseRow)) {
00161:    $processedFile = $this->createDomainObject($databaseRow);
00162:   } else {
00163:    $processedFile = $this->createNewProcessedFileObject($file, $taskType, $configuration);

9 TYPO3\CMS\Core\Resource\ProcessedFileRepository::findOneByOriginalFileAndTaskTypeAndConfiguration(TYPO3\CMS\Core\Resource\File, "Image.Preview", array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/Service/FileProcessingService.php:
00091:   $processedFileRepository = Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Resource\\ProcessedFileRepository');
00092: 
00093:   $processedFile = $processedFileRepository->findOneByOriginalFileAndTaskTypeAndConfiguration($fileObject, $taskType, $configuration);
00094: 
00095:   // set the storage of the processed file

8 TYPO3\CMS\Core\Resource\Service\FileProcessingService::processFile(TYPO3\CMS\Core\Resource\File, TYPO3\CMS\Core\Resource\ResourceStorage, "Image.Preview", array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/ResourceStorage.php:
00788:    throw new \InvalidArgumentException('Cannot process files of foreign storage', 1353401835);
00789:   }
00790:   $processedFile = $this->getFileProcessingService()->processFile($fileObject, $this, $context, $configuration);
00791: 
00792:   return $processedFile;

7 TYPO3\CMS\Core\Resource\ResourceStorage::processFile(TYPO3\CMS\Core\Resource\File, "Image.Preview", array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/File.php:
00291:   */
00292:  public function process($taskType, array $configuration) {
00293:   return $this->getStorage()->processFile($this, $taskType, $configuration);
00294:  }
00295: 

6 TYPO3\CMS\Core\Resource\File::process("Image.Preview", array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/Classes/FileList.php:
00620:        // Thumbnails?
00621:        if ($this->thumbs && $this->isImage($ext)) {
00622:         $processedFile = $fileObject->process(\TYPO3\CMS\Core\Resource\ProcessedFile::CONTEXT_IMAGEPREVIEW, array());
00623:         if ($processedFile) {
00624:          $thumbUrl = $processedFile->getPublicUrl(TRUE);

5 TYPO3\CMS\Filelist\FileList::formatFileList(array, "file")

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/Classes/FileList.php:
00351:    }
00352:    // Files are added
00353:    $iOut .= $this->formatFileList($files, $titleCol);
00354:    // Header line is drawn
00355:    $theData = array();

4 TYPO3\CMS\Filelist\FileList::getTable("fileext,tstamp,size,rw,_REF_")

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/Classes/FileList.php:
00197:   */
00198:  public function generateList() {
00199:   $this->HTMLcode .= $this->getTable('fileext,tstamp,size,rw,_REF_');
00200:  }
00201: 

3 TYPO3\CMS\Filelist\FileList::generateList()

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/Classes/Controller/FileListController.php:
00288:    $this->filelist->start($this->folderObject, $this->pointer, $this->MOD_SETTINGS['sort'], $this->MOD_SETTINGS['reverse'], $this->MOD_SETTINGS['clipBoard'], $this->MOD_SETTINGS['bigControlPanel']);
00289:    // Generate the list
00290:    $this->filelist->generateList();
00291:    // Write the footer
00292:    $this->filelist->writeBottom();

2 TYPO3\CMS\Filelist\Controller\FileListController::main()

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/mod1/index.php:
00044: $SOBE = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Filelist\\Controller\\FileListController');
00045: $SOBE->init();
00046: $SOBE->main();
00047: $SOBE->printContent();
00048: ?>

1 require("/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/mod1/index.php")

/usr/local/src/typo3/TYPO3_6-1/typo3/mod.php:
00039:  require $temp_path . 'conf.php';
00040:  $BACK_PATH = '';
00041:  require $temp_path . 'index.php';
00042:  $isDispatched = TRUE;
00043: } else {

#5

Updated by Ernesto Baschny about 8 years ago

  • Status changed from New to Accepted

#48336 introduced (mis)using the "deleted" flag. It was included in 6.0.7 and 6.1.2.

This was later reverted in 6.0.8 and 6.1.3.

So if you had one of 6.0.7 or 6.1.2 installed for some time you might have gotten "deleted" flag being set.

I propose that we remove the flag that was introduced as a misconception and stick to the rules set up in task #50876 which is what we decided in FAL Code Sprint in Mainz. So we should backport the removal of this flag also in 6.1 and 6.0. I don't expect "extensions to break". If yes, they have to adapt, because it's not worth really to support two separate "ways" of dealing with it.

#6

Updated by Gerrit Code Review about 8 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23567

#7

Updated by Ernesto Baschny about 8 years ago

  • Parent task set to #50876
#8

Updated by Ernesto Baschny about 8 years ago

  • Subject changed from sys_file records marked as deleted crash file list to sys_file records marked as deleted crash file list (AKA: remove sys_file.deleted flag)
#9

Updated by Ernesto Baschny about 8 years ago

  • Assignee set to Ernesto Baschny
  • Is Regression set to No
#10

Updated by Gerrit Code Review about 8 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23567

#11

Updated by Gerrit Code Review about 8 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23567

#12

Updated by Stefan Neufeind about 8 years ago

!!! still needs a follow-up for 6.2 to remove the field completely

#13

Updated by Stefan Neufeind about 8 years ago

removing that field was included in the patch - will take that into account for the backports then

#14

Updated by Gerrit Code Review about 8 years ago

Patch set 1 for branch TYPO3_6-1 has been pushed to the review server.
It is available at https://review.typo3.org/23742

#15

Updated by Ernesto Baschny about 8 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#16

Updated by Gerrit Code Review about 8 years ago

  • Status changed from Resolved to Under Review

Patch set 2 for branch TYPO3_6-1 has been pushed to the review server.
It is available at https://review.typo3.org/23742

#17

Updated by Gerrit Code Review about 8 years ago

Patch set 1 for branch TYPO3_6-0 has been pushed to the review server.
It is available at https://review.typo3.org/23748

#18

Updated by Ernesto Baschny about 8 years ago

  • Status changed from Under Review to Resolved
#19

Updated by Gerrit Code Review about 8 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch TYPO3_6-0 has been pushed to the review server.
It is available at https://review.typo3.org/23758

#20

Updated by Gerrit Code Review about 8 years ago

Patch set 1 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23759

#21

Updated by Gerrit Code Review about 8 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23759

#22

Updated by Gerrit Code Review about 8 years ago

Patch set 1 for branch TYPO3_6-1 has been pushed to the review server.
It is available at https://review.typo3.org/23760

#23

Updated by Gerrit Code Review about 8 years ago

Patch set 1 for branch TYPO3_6-0 has been pushed to the review server.
It is available at https://review.typo3.org/23761

#24

Updated by Anja Leichsenring about 8 years ago

  • Status changed from Under Review to Resolved
#25

Updated by Benni Mack about 3 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF