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

Check if the path given is already an absolute path #12948

Closed
wants to merge 1 commit into from

Conversation

michelbrito
Copy link

Description

The Magento\Framework\Filesystem\Driver\File::isFile function returns false, because it concatenates the symlink path with the absolute path in getAbsolutePath.

Fixed Issues

  1. Invalid template files for every vendor templates #8368: Invalid template files for every vendor templates

Manual testing scenarios

  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/).

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jan 2, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team magento-engcom-team added bugfix Area: Frontend 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 Jan 2, 2018
@orlangur
Copy link
Contributor

orlangur commented Jan 3, 2018

https://github.com/magento/magento2/blob/2.2-develop/app/etc/vendor_path.php exists to customize vendor path, why symlinking is needed?

Proposed implementation opens a huge security breach as current implementation is written in such a way that no file must be accessed outside base path.

@sidolov
Copy link
Contributor

sidolov commented Jun 2, 2018

@michelbrito , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for collaboration

@sidolov sidolov closed this Jun 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Frontend bugfix Progress: needs update Release Line: 2.2 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants