Project

General

Profile

Actions

Bug #66397

closed

FAL ignores but fails on missing "sha1" for entries

Added by Claus Due over 9 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
File Abstraction Layer (FAL)
Target version:
-
Start date:
2015-04-13
Due date:
% Done:

100%

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

Description

This one is not so much a bug as a major inconvenience caused by errors being hidden away deep inside FAL. It cost me a lot of hours. It is relevant in all TYPO3 versions from 6.2 and up.

The sequence of unfortunate events that lead to this problem, starting from the bottom up:

  • The SQL schema for `sys_file` does not allow a NULL value for column "sha1" (correct behavior!)
  • The dispatching of this SQL query is not checked for errors (also reasonably correct behavior - it should imho be safe to assume at this point that data is good)
  • The SQL server is configured for strict mode and thus fails on NULL values; however, non-strict mode would also fail except then there would be no warning from SQL (which I'd consider quite bad)

That's the SQL related stuff; only exposed because my SQL runs strict mode. Further up in the call stack:

  • The FAL Indexer extracts information via the Driver but extracts the "hash" via a dedicated method
  • This method's return value is never checked, simply assigned to the array that gets passed.

And that's where my troubles began: my particular FAL driver was receiving an unexpected NULL value for hash from the repository it uses - and this caused the following things to break down only partially with no easy way to determine the cause:

  • All files and folders had uid=0
  • One metadata record was being written and reused for every file because it was also referenced with uid=0
  • File deletions would report an error but the file would be deleted. The error caused by "trying to delete metadata with uid=0"
  • Unable to rename files because the rename file function expects the UID of the sys_file record.
  • Thumbnails not generated because files were refererenced by their uid which then loaded the file by identifier - naturally, this broke because the uid was zero.
  • Files not downloadable, same as thumbnailer, because dumpFile eID script expects a UID.
  • Files not detected as updated because hashes always match (technically, NULL === NULL so that's a "match")
  • Files impossible to reference from any uploader, IRRE or otherwise.

All while no errors were being reported except for the almost invisible SQL failure, sending me on a two-day rabbit hunt.

I came up with the following two suggested ways to relieve this pain for future developers:

1. Check the SQL error when Indexer inserts a record. However, this is only effective in strict mode.
2. Throw a specific exception in Indexer when for whatever reason, generating a "hash" via the driver's dedicated method returns an invalid value (bool, null, integer, object-not-string, etc).

I'd recommend the second solution because it covers more possible ENV setups and is much more explicit about the reason it failed.

Actions #1

Updated by Riccardo De Contardi about 9 years ago

  • Category set to File Abstraction Layer (FAL)
Actions #2

Updated by Gerrit Code Review over 5 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59893

Actions #3

Updated by Gerrit Code Review over 5 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59893

Actions #4

Updated by Gerrit Code Review over 5 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59893

Actions #5

Updated by Gerrit Code Review over 5 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59893

Actions #6

Updated by Gerrit Code Review about 5 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59893

Actions #7

Updated by Gerrit Code Review about 5 years ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62230

Actions #8

Updated by Anonymous about 5 years ago

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

Updated by Benni Mack almost 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF