Project

General

Profile

Actions

Bug #56999

closed

broken handling of rawurlencoded filepaths in FAL's resource-factory

Added by Stephan Jorek over 10 years ago. Updated over 10 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
File Abstraction Layer (FAL)
Target version:
-
Start date:
2014-03-17
Due date:
% Done:

0%

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

Description

The FAL-Implementaion contains the following check in TYPO3\CMS\Core\Resource\ResourceFactory->retrieveFileOrFolderObject, to determine the existence of the given $input as file:

…
                } elseif (@is_file(PATH_site . $input)) {
                        // only the local file
                        return $this->getFileObjectFromCombinedIdentifier($input);
                } else {
…

As the Resource-FilePath-Sanatizer applies rawurlencode to all filepaths, the is_file function-call always fails for file-paths which contain whitespaces (or other characters which change during url-encoding). Hence if the given input is something like '/fileadmin/path%20to%20file.extension' (notice the two spaces => %20) it will silently fail and throw a misleading folder-not-found exception instead.

To solve this bug we could rawurldecode the given $input - but this makes only sense if we always deal with local files only:

diff --git a/typo3/sysext/core/Classes/Resource/ResourceFactory.php b/typo3/sysext/core/Classes/Resource/ResourceFactory.php
index 9a10e8b..98deedb 100644
--- a/typo3/sysext/core/Classes/Resource/ResourceFactory.php
+++ b/typo3/sysext/core/Classes/Resource/ResourceFactory.php
@@ -438,7 +438,7 @@ class ResourceFactory implements \TYPO3\CMS\Core\SingletonInterface {
        public function retrieveFileOrFolderObject($input) {
                // Remove PATH_site because absolute paths under Windows systems contain ':'
                // This is done in all considered sub functions anyway
-               $input = str_replace(PATH_site, '', $input);
+               $input = str_replace(PATH_site, '', rawurldecode($input));

                if (GeneralUtility::isFirstPartOfStr($input, 'file:')) {
                        $input = substr($input, 5);

(This patch is also attached to this bug-report).

I appreciate any feedback,
Cheerio,
Stephan Jorek


Files

patch-typo3-6.2-fal-rawurlencode-bug.diff (855 Bytes) patch-typo3-6.2-fal-rawurlencode-bug.diff patch to rawurldecode given $input in TYPO3\CMS\Core\Resource\ResourceFactory-> Stephan Jorek, 2014-03-17 16:57
Actions #1

Updated by Markus Klein over 10 years ago

  • Status changed from New to Needs Feedback

Can you please tell us, how you exactly use this API?
How/Where do you call retrieveFileOrFolderObject()?

Actions #2

Updated by Stephan Jorek over 10 years ago

Of course i can, but while I'm writing this feedback, I discovered that this might be our fault … but please read on. I highlighted our potential fault in the TypoScript given below as embedded comment and tried to give the correct solution.

Short Version:

We don't call this API directly, we use the FILES-ContentObject with a IMAGE-renderObj in TypoScript and render this TypoScript with our freshly migrated TYPO3 6.2 Plugin which formerly was a TYPO3 4.x piBase-extension.

Long Version:

We use it in a \TYPO3\CMS\Frontend\Plugin\AbstractPlugin under TYPO3 6.2 (git, current master), which was originally developed as a TYPO3 4.x piBase-plugin. The extension has been rewritten for TYPO3 6.2, without switching to extbase, please don't ask :-). The “sys_file”-table was originally the one from the “dam” extension and has been completely re-indexed using TYPO3 6.2 FAL. Its file-references have been reimported, partially by hand to verify that our scripts are working and partially using a fork of https://github.com/fnagel/t3ext-dam_falmigration. The problematic file reference was a “fresh and new” one, so I don't expect an accident during mass-import of “dam” data here … or am I wrong ? :-)

The following an anonymized and simplified excerpt from our sources:

…
plugin.tx_extensionkey_theplugin = USER
plugin.tx_extensionkey_theplugin {
…
  table = tx_extenionkey_table_with_fal_references
…
  renderObj = FILES
  renderObj {
    references {
      table = tx_extenionkey_table_with_fal_references
      uid.field = uid
      fieldName = field_with_fal_reference
    }

    renderObj = COA
…
    renderObj.100 = IMAGE
    renderObj.100 {
      # HERE COMES OUR POTENTIAL FAULT:
      file.import.data = file:current:publicUrl

      # So after reading the related sources under
      # https://git.typo3.org/Packages/TYPO3.CMS.git/blob/master:/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php#l5207
      # I guess the following is the correct solution:
      #
      #   file.import.data = file:current:uid
      #
      # If I'm right, than shame on me.  I assumed that somewhere in FAL some internal
      # url-encoding happens (due to some non-local FAL-drivers or something like that).
    }
…
  }
}
…

But, to complete the above TypoScript here the corresponding (simplified) PHP-Code, which renders the TypoScript and works quite similar to TYPO3\CMS\Frontend\ContentObject\ContentContentObject :

<?php

namespace Vendor\ExtensionKey\Plugin;

class ThePlugin extends \TYPO3\CMS\Frontend\Plugin\AbstractPlugin {
…
  /**
    * @var TYPO3\CMS\Frontend\ContentObject
    */
  protected var $cObj;

  public main($content, array $conf = array()) {
    …

    $renderObjName = $conf['renderObj'];
    $renderObjConf = $conf['renderObj.'];
    $renderObjKey = 'plugin.tx_extensionkey_theplugin.renderObj';

    …
    do {
      /** @var $cObj \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer */
      $cObj = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Frontend\\ContentObject\\ContentObjectRenderer');
      $cObj->setParent($this->cObj->data, $this->cObj->currentRecord);
      $this->cObj->currentRecordNumber = 0;
      $cobjValue = '';
      while ($row = $GLOBALS['TYPO3_DB']->sql_fetch_assoc($res)) {
        …

        $this->cObj->currentRecordNumber++;
        $cObj->parentRecordNumber = $this->cObj->currentRecordNumber;
        $GLOBALS['TSFE']->currentRecord = $conf['table'] . ':' . $row['uid'];
        $this->cObj->lastChanged($row['tstamp']);
        $cObj->start($row, $conf['table']);
        $tmpValue = $cObj->cObjGetSingle($renderObjName, $renderObjConf, $renderObjKey);
        $content .= $tmpValue;
        …
      }
    } while($something);
    …
    return $content;
  } 
…
}

I'll check my assumption tomorrow and give you asap more feedback here. Thanks for your immediate reply anyway. And if you read everything above, than tanks for your patience too. ;-)

PS.: Maybe it's a good idea to add a code-sample to http://docs.typo3.org/typo3cms/TyposcriptReference/Functions/Imgresource/Index.html under “import” as soon as TYPO3 6.2 has been released officially.

Actions #3

Updated by Steffen Ritter over 10 years ago

Hi thanks for you feedback; passing publicUrl is indeed wrong in will lead to these problems
as the name tells: it is an URL

Actions #4

Updated by Stephan Jorek over 10 years ago

Hey,

just for the Record, the final solution is:

…
plugin.tx_extensionkey_theplugin = USER
plugin.tx_extensionkey_theplugin {
…
  table = tx_extenionkey_table_with_fal_references
…
  renderObj = FILES
  renderObj {
    references {
      table = tx_extenionkey_table_with_fal_references
      uid.field = uid
      fieldName = field_with_fal_reference
    }

    renderObj = COA
…
    renderObj.100 = IMAGE
    renderObj.100 {
      file.import.data = file:current:uid
      file.treatIdAsReference = 1
    }
…
  }
}
…

We had to enable the flag “treatIdAsReference”, as “file:current:uid” delivers the uid from the sys_file_reference-Table … weird, but working.

So far,
Stephan Jorek

P.S. This ticket is resolved (for us).

Actions #5

Updated by Alexander Opitz over 10 years ago

  • Status changed from Needs Feedback to Closed
  • Target version deleted (6.2.0)

Thanks for feedback.

Actions

Also available in: Atom PDF