numero2 / contao-proper-filenames

Replaces special characters in filenames right after upload.
https://www.numero2.de/contao/erweiterungen/proper-filenames.html
GNU Lesser General Public License v3.0
13 stars 5 forks source link

Path "…/php8C41.tmp" is not inside the Contao root dir #9

Closed fritzmg closed 3 years ago

fritzmg commented 3 years ago

With the addition of the validateFormField hook in https://github.com/numero2/contao-proper-filenames/commit/76b317218cc40cbd7349277910c1edce094ccb25 the following error will now appear in the front end (in Contao 4.4 at least):

InvalidArgumentException:
Path "…/php8C41.tmp" is not inside the Contao root dir

  at vendor\contao\core-bundle\src\Resources\contao\library\Contao\StringUtil.php:1102
  at Contao\StringUtil::stripRootDir('…/php8C41.tmp')
     (vendor\numero2\contao-proper-filenames\src\Resources\contao\classes\CheckFilenames.php:104)
  at numero2\ProperFilenames\CheckFilenames->renameFormUploads(object(FormFileUpload), 'auto_form_39', …)
     (vendor\contao\core-bundle\src\Resources\contao\forms\Form.php:213)
  at Contao\Form->compile()
     (vendor\contao\core-bundle\src\Resources\contao\classes\Hybrid.php:235)
  at Contao\Hybrid->generate()
     (vendor\contao\core-bundle\src\Resources\contao\forms\Form.php:88)
  at Contao\Form->generate()
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Controller.php:477)
  at Contao\Controller::getContentElement(object(ContentModel), 'main')
     (vendor\contao\core-bundle\src\Resources\contao\modules\ModuleArticle.php:182)
  at Contao\ModuleArticle->compile()
     (vendor\contao\core-bundle\src\Resources\contao\modules\Module.php:219)
  at Contao\Module->generate()
     (vendor\contao\core-bundle\src\Resources\contao\modules\ModuleArticle.php:64)
  at Contao\ModuleArticle->generate(false)
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Controller.php:417)
  at Contao\Controller::getArticle(object(ArticleModel), false, false, 'main')
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Controller.php:279)
  at Contao\Controller::getFrontendModule('0', 'main')
     (vendor\contao\core-bundle\src\Resources\contao\pages\PageRegular.php:176)
  at Contao\PageRegular->prepare(object(PageModel))
     (vendor\contao\core-bundle\src\Resources\contao\pages\PageRegular.php:46)
  at Contao\PageRegular->getResponse(object(PageModel), true)
     (vendor\contao\core-bundle\src\Resources\contao\controllers\FrontendIndex.php:308)
  at Contao\FrontendIndex->renderPage(object(Collection))
     (vendor\contao\core-bundle\src\Resources\contao\controllers\FrontendIndex.php:74)
  at Contao\FrontendIndex->run()
     (vendor\contao\core-bundle\src\Controller\FrontendController.php:42)
  at Contao\CoreBundle\Controller\FrontendController->indexAction()
     (vendor\symfony\symfony\src\Symfony\Component\HttpKernel\HttpKernel.php:151)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor\symfony\symfony\src\Symfony\Component\HttpKernel\HttpKernel.php:68)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor\symfony\symfony\src\Symfony\Component\HttpKernel\Kernel.php:200)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (web\app_dev.php:73)

I am not sure the current code can ever work, because tmp_name is, as the name implies, a temporary location which might be somewhere outside the project root and thus StringUtil::stripRootDir will always fail.

I would propose the following changes:

  1. Rework CheckFilenames::renameFiles so that it can work with both absolute and project root relative file paths.
  2. Use Symfony Filesystem instead of legacy Contao\Files.

I can provide an according PR.

bennyborn commented 3 years ago

I'm not quite sure if there even is a bug.

Did you use the Cores "FormFileUpload" field or is this a custom form widget?

The Cores widget should always report a tmp_name that's inside TL_ROOT:

        // Add the session entry (see #6986)
    $_SESSION['FILES'][$this->strName] = array
    (
        'name'     => $file['name'],
        'type'     => $file['type'],
        'tmp_name' => TL_ROOT . '/' . $strFile,
        'error'    => $file['error'],
        'size'     => $file['size'],
        'uploaded' => true,
        'uuid'     => $strUuid
    );
fritzmg commented 3 years ago

Good point. Yes we do use the regular form upload. I'll debug the issue further why the tmp_name is not within TL_ROOT in our case.