Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid template files for every vendor templates #8368

Closed
lumiru opened this issue Feb 1, 2017 · 7 comments
Closed

Invalid template files for every vendor templates #8368

lumiru opened this issue Feb 1, 2017 · 7 comments
Labels
Area: Frontend Event: distributed-cd Distributed Contribution Day Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed non-issue

Comments

@lumiru
Copy link

lumiru commented Feb 1, 2017

Preconditions

  1. Magento 2.1.3
  2. PHP 7.0.15

Steps to reproduce

  1. Put your vendor folder in an other directory (e.g., in /srv/magento/vendor).
  2. Create a symbolic link for vendor to its new path (e.g. ln -s /srv/magento/vendor /var/www/vendor).
  3. Try to load any website page (e.g. http://example.com/admin/).

Expected result

  1. The website should load as always (or a configuration should permit symlinks usage).

Actual result

  1. Magento cannot load theme files because they are not valid (we got a line in system.log for each needed template):
[2017-02-01 11:59:38] main.CRITICAL: Invalid template file: '/var/www/htdocs/shared/vendor/magento/module-backend/view/adminhtml/templates/admin/login.phtml' in module: 'Magento_Backend' block's name: 'admin.login' [] []

What has happened?

When I debug this issue, I found $this->isPathInDirectories($filename, $this->moduleDirs) returns true but $this->getRootDirectory()->isFile($this->getRootDirectory()->getRelativePath($filename)) returns false in Magento\Framework\View\Element\Template\File\Validator::isValid, whether the file exists or not.
When I go deeper, I see return $this->driver->isFile($this->driver->getAbsolutePath($this->path, $path)); in Magento\Framework\Filesystem\Directory\Read::isFile($path). So the complete parameter for Magento\Framework\Filesystem\Driver\File:isFile is File::getAbsolutePath('/var/www', File::getRelativePath('/var/www/', '/srv/magento/vendor/magento/module-backend/view/adminhtml/templates/admin/login.phtml'));.

Why not. The issue comes from File::getAbsolutePath. Its behaviour is the same if the $path parameter is leaded by a / or not:

File::getAbsolutePath('/var/www/', '/srv/magento/vendor/magento/module-backend/view/adminhtml/templates/admin/login.phtml') # -> '/var/www/srv/magento/vendor/magento/module-backend/view/adminhtml/templates/admin/login.phtml'

However, we cannot easily change this behaviour because it creates new side effects (because some modules uses this feature).

Another way to fix this issue is to modify the File::getRelativePath function. Indeed, this function does not returns a relative path if target $path is not leaded by $basePath:

File::getRelativePath('/var/www/', '/srv/magento/vendor/magento/module-backend/view/adminhtml/templates/admin/login.phtml') # -> '/srv/magento/vendor/magento/module-backend/view/adminhtml/templates/admin/login.phtml'

The simplest way to fix it is to add as double dots at start as necessary. For example:

    public function getRelativePath($basePath, $path = null)
    {
        $path = $this->fixSeparator($path);
        if (strpos($path, $basePath) === 0 || $basePath == $path . '/') {
            $result = substr($path, strlen($basePath));
        } else {
            $str = '';
            for($i = substr_count($basePath, '/'); $i > 1; --$i)
                $str .= '../';
            $result = $str . ltrim($path, '/');
        }
        return $result;
    }

But I encourage you to prefer a complete implementation of getRelativePath: http://stackoverflow.com/questions/2637945/getting-relative-path-from-absolute-path-in-php

@slackerzz
Copy link
Member

I found the same bug but in a different scenario.
My media is a symbolic link to an external folder since i'm using capistrano-magento2 and it is impossible to delete a folder/file throught the wysiwyg

@IlnitskiyArtem
Copy link

Hi, @lumiru. Internal ticket MAGETWO-71178, which tracks this GitHub issue, is in our issue backlog.

@veloraven veloraven added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Aug 7, 2017
@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Area: Frontend Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Sep 28, 2017
@magento-engcom-team
Copy link
Contributor

@lumiru, thank you for your report.
We've created internal ticket(s) MAGETWO-71178 to track progress on the issue.

@magento-engcom-team magento-engcom-team added 2.2.x Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Oct 11, 2017
@okorshenko okorshenko removed the 2.1.x label Dec 14, 2017
@orlangur
Copy link
Contributor

orlangur commented Jan 3, 2018

At first sight it looks like there is nothing to fix here.

No files must be accessed outside of Magento base path and this behavior is intentional. Otherwise in case of a mistake in some module code it would be possible to read sensitive system files.

Its behaviour is the same if the $path parameter is leaded by a / or not:

Read a path starting from slash is never a good idea.

Is there any use case not covered by https://magento.stackexchange.com/a/95095 answer?

@magento-engcom-team magento-engcom-team added the Event: distributed-cd Distributed Contribution Day label Mar 19, 2018
@dnna
Copy link

dnna commented Aug 18, 2018

I also encountered this issue while trying to move the vendor dir to a path outside Magento's root (/app-vendor specifically) in my Docker container. I understand the security reason for this restriction, but should we maybe allow it to have access to templates inside the vendor dir?

@tschirmer
Copy link

There are a number of reasons for moving the vendor directory outside of the Magento Root. It's worth seeing if the can be specified as an option, just like the directories specified in the $_SERVER variables:

e.g.:
$_SERVER["MAGE_DIRS"]['code']["path"]
$_SERVER["MAGE_DIRS"]['di']["path"]
$_SERVER["MAGE_DIRS"]['generated']["path"]

In testing environments, it's quite easy to setup a shared codebase, with multiple databases (just change the $_SERVER["MAGE_DIRS"]['etc']["path"] for debugging different db configuration variables). But using that causes a lot of issues with media writing (it relys on the BP to be unaltered). We've found it troublesome to try and run that without modification, and we run into vendor validation issues the moment we change the base path outside of where the vendor directory is.

@orlangur orlangur removed bug report Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development labels Feb 1, 2019
@orlangur
Copy link
Contributor

orlangur commented Feb 1, 2019

Current behavior is intentional and it should not be changed, there were a bunch of such reports before, for example, #15252 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Frontend Event: distributed-cd Distributed Contribution Day Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed non-issue
Projects
None yet
Development

No branches or pull requests