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

Images can't be uploaded using WYSIWYG if media directory is a symlink #13929

Closed
erikhansen opened this issue Mar 2, 2018 · 44 comments
Closed
Labels
Event: balance-cd Event: distributed-cd Distributed Contribution Day Event: kiev-cd Fixed in 2.3.x The issue has been fixed in 2.3 release line 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 Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development 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

Comments

@erikhansen
Copy link
Contributor

erikhansen commented Mar 2, 2018

As of 2.2.3, it's not possible to upload images via the WYSIWYG if the media directory is a symlink. This is due to this new file.

Preconditions

  1. Install Magento 2.2.3
  2. Ensure that the media directory is a symlink. For example, the pub/media directory should be a symlink that points somewhere else.

Steps to reproduce

  1. Edit a CMS Block and click "Insert Image".
  2. Try to upload an image.

Expected result

  1. The image should upload successfully.

Actual result

  1. You'll get an error like this:
    Directory /var/www/prod/releases/20180228212716/pub/media/wysiwyg is not under storage root path.
    
    magento admin-xazv0

Workaround

UPDATE: As of Magento 2.2.5 / 2.3.0, this issue has been fixed and you shouldn't apply the patch below or else you'll break the functionality.

We've temporarily fixed this by changing this line from this:

$realPath = realpath($path);
$root = $this->directoryList->getPath($directoryConfig);

to this (note the addition of the realpath function call):

$realPath = realpath($path);
// BEGIN EDIT
/**
 * Since media directory is a symlink, need to run both paths through realpath in order for the comparison to
 * work.
 * The proper fix for this should involve a STORE > Configuration setting where an admin can choose whether to
 * allow symlinked directories.
 */
$root = realpath($this->directoryList->getPath($directoryConfig));
// END EDIT
@magento-engcom-team magento-engcom-team added Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Fixed in 2.3.x The issue has been fixed in 2.3 release line 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 labels Mar 2, 2018
@magento-engcom-team
Copy link
Contributor

@erikhansen, thank you for your report.
The issue is already fixed in 2.3.0
But we will consider to backport the fix to patch releases

@barryvdh
Copy link
Contributor

barryvdh commented Mar 5, 2018

Thank you, this is an important issue for us also. Is there a patch available for 2.2.3?

@hostep
Copy link
Contributor

hostep commented Mar 5, 2018

@magento-engcom-team: can you point us to the commit where this is fixed in 2.3-develop? I have the feeling that we will run into the same problems due to the way how we deploy to the servers.

@kanduvisla
Copy link
Contributor

I can confirm this behavior as well in 2.1.12. Can the backport also be considered for 2.1.x?

@mikelevy300
Copy link

I'm having the same issue on 2.1.12.

@JosephMaxwell
Copy link
Contributor

JosephMaxwell commented Mar 7, 2018

Just ran into this issue, too. Here is the code I used to fix it (temporarily):

etc/di.xml

<?xml version="1.0" ?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <preference for="Magento\Framework\App\Filesystem\DirectoryResolver"
                type="\ModuleVendor\ModuleName\App\Filesystem\DirectoryResolver"/>
</config>

Override/App/Filesystem/DirectoryResolver.php

<?php
declare(strict_types=1);

/**
 * TODO: temporary override until patch or Magento 2.3 is released.
 * https://github.com/magento/magento2/issues/13929
 */
namespace ModuleVendor\ModuleName\Override\App\Filesystem;

use Magento\Framework\App\Filesystem\DirectoryList;

/**
 * Magento directories resolver.
 */
class DirectoryResolver
{
    /**
     * @var DirectoryList
     */
    private $directoryList;

    /**
     * @param DirectoryList $directoryList
     */
    public function __construct(DirectoryList $directoryList)
    {
        $this->directoryList = $directoryList;
    }

    /**
     * Validate path.
     *
     * Gets real path for directory provided in parameters and compares it with specified root directory.
     * Will return TRUE if real path of provided value contains root directory path and FALSE if not.
     * Throws the \Magento\Framework\Exception\FileSystemException in case when directory path is absent
     * in Directories configuration.
     *
     * @param string $path
     * @param string $directoryConfig
     * @return bool
     * @throws \Magento\Framework\Exception\FileSystemException
     */
    public function validatePath($path, $directoryConfig = DirectoryList::MEDIA)
    {
        $realPath = realpath($path);
        // BEGIN EDIT by @erikhansen
        /**
         * Since media directory is a symlink, need to run both paths through realpath in order for the comparison to
         * work.
         * The proper fix for this should involve a STORE > Configuration setting where an admin can choose whether to
         * allow symlinked directories.
         */
        $root = realpath($this->directoryList->getPath($directoryConfig));
        // END EDIT

        return strpos($realPath, $root) === 0;
    }
}

@yanncharlou
Copy link

@JosephMaxwell there's a typo in namespace in etc/di.xml
You should add \Override\

<?xml version="1.0" ?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <preference for="Magento\Framework\App\Filesystem\DirectoryResolver"
                type="\ModuleVendor\ModuleName\Override\App\Filesystem\DirectoryResolver"/>
</config>

@osrecio
Copy link
Member

osrecio commented Mar 21, 2018

I solved with cweagans/composer-patches.
I created a patch (for Magento Composer projects):
name: 13929_2.2.3_directory_resolver_composer_v1.patch

Index: magento/vendor/magento/framework/App/Filesystem/DirectoryResolver.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- magento/vendor/magento/framework/App/Filesystem/DirectoryResolver.php	(date 1521531005000)
+++ magento/vendor/magento/framework/App/Filesystem/DirectoryResolver.php	(date 1521561205000)
@@ -39,7 +39,7 @@
     public function validatePath($path, $directoryConfig = DirectoryList::MEDIA)
     {
         $realPath = realpath($path);
-        $root = $this->directoryList->getPath($directoryConfig);
+        $root = realpath($this->directoryList->getPath($directoryConfig));
         
         return strpos($realPath, $root) === 0;
     }

@andrewhowdencom
Copy link
Contributor

This presumably broke as part of the security patches (there's a bunch of stuff associated with path disclosures and RCEs in the 2.2.3 release). Anyone got any idea why this change was made? (so we don't set about reintroducing it with third party fixes)

@piotrekkaminski
Copy link
Contributor

@magento-engcom-team

The issue is already fixed in 2.3.0
According to your previous comment it is fixed. I don't see it being fixed. Can you point out to commit/Jira issue with the fix?

@andrewhowdencom
Copy link
Contributor

andrewhowdencom commented Mar 23, 2018

Obligatory: https://github.com/sitewards/magento2-fix13929

@mikewhitby mikewhitby self-assigned this Mar 24, 2018
@mikewhitby
Copy link
Contributor

I'm working on it #distributed-cd

mikewhitby added a commit to mikewhitby/magento2 that referenced this issue Mar 24, 2018
…ory is a symlink

- Added realpath() to root dir to resolve symlinks
- Added tests
@erikhansen
Copy link
Contributor Author

@andrewhowdencom - For the record, I much prefer to fix core issues like this using the cweagans/composer-patches approach that @osrecio mentioned above rather than using a preference. The reason for this is that if the underlying code changes in a future version of Magento (which is likely in this situation), the composer install will fail to complete which will alert you to the fact that the patch needs to be removed or changed to work with the latest code. This is assuming that you have the composer-exit-on-patch-failure option set to true.

@andrewhowdencom
Copy link
Contributor

@andrewhowdencom - For the record, I much prefer to fix core issues like this using the cweagans/composer-patches approach that @osrecio mentioned above rather than using a preference

Yah that seems reasonable ^^ I packaged it as an extension for easy distribution so it hooks into our existing mechanisms for discovery by our team internally (namely, satis / bitbucket). I could further restrict it to the specific version of Magento (actually @mzeis spotted that) but it's a dirty temporary fix until core addresses it -- I hadn't put too much thought into managing it long term.

@p-bystritsky
Copy link
Contributor

Issue is reproducible on 2.3-develop branch.

@p-bystritsky p-bystritsky added Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release and removed Fixed in 2.3.x The issue has been fixed in 2.3 release line labels Apr 3, 2018
@magento-engcom-team magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Apr 3, 2018
@magento-engcom-team
Copy link
Contributor

@erikhansen, thank you for your report.
We've acknowledged the issue and added to our backlog.

dangron pushed a commit to Space48/magento2-patches that referenced this issue Jun 27, 2018
@clemblanco
Copy link

Is this fixed for 2.2.4 yet?

@MidnightDesign
Copy link

@clementblanco It's fixed in 2.2.5.

@mikehenze
Copy link

Still have this problem on 2.2.5

@tuyennn
Copy link
Member

tuyennn commented Jul 11, 2018

Same this problem on 2.1.14

@aKalisch
Copy link

@magento-engcom-team i have the same issue as @digvijay2017.

File: Magento\Cms\Model\Wysiwyg\Images\Storage

    /**
     * Is path under storage root directory
     *
     * @param string $path
     * @return void
     * @throws \Magento\Framework\Exception\LocalizedException
     */
    protected function _validatePath($path)
    {
        $root = $this->_sanitizePath($this->_cmsWysiwygImages->getStorageRoot());
        if ($root == $path) {
            throw new \Magento\Framework\Exception\LocalizedException(
                __('We can\'t delete root directory %1 right now.', $path)
            );
        }
        if (strpos($path, $root) !== 0) {
            throw new \Magento\Framework\Exception\LocalizedException(
                __('Directory %1 is not under storage root path.', $path)
            );
        }
    }

@maximbaibakov
Copy link

Just tested this on 2.2-develop branch and it has been fixed there already.
Tested this on 2.2.5 and it has been fixed there.

There is a commit for a reference: a6afb59

If someone still experience the issue, please provide more details

@TehJackal
Copy link

I've just recently installed 2.2.5 in the past week after this was closed 9 days ago.

I am still experiencing the issue with image uploads.

@maximbaibakov
Copy link

Hi @TehJackal

Could you please explain what is your setup?
What error do you see?
What is your symlink path?

Regards,
Maxim

@TehJackal
Copy link

I'm not sure what you mean by Symlink path, as I'm still fairly new to using and working with Magento 2

That being said when I try to upload a simple Jpg file, it does a refresh icon and then gives me this.

image

@hostep
Copy link
Contributor

hostep commented Aug 21, 2018

@TehJackal: that's a completely different error then what is discussed in this thread and has indeed nothing to do with symlinks. You might find a solution in the following thread though: #16531 (try installing the php extension 'fileinfo')

@ChrHartwig
Copy link

ChrHartwig commented Oct 30, 2018

This error is fixed in 2.2.5

@tushhan
Copy link

tushhan commented Apr 20, 2020

Hi, This problem occurs on Magento 2.3.4 as well.

1- File validation fails if upload an image
2- Sub folders shows an error as below.


Fatal error: Uncaught Error: Call to undefined function mime_content_type() in
.....
/vendor/magento/module-cms/Model/Wysiwyg/Images/Storage.php on line 362

After testing on several PHP versions, I found that the problem is from PHP versions of 7.2 and 7.3.
As it looks like working on PHP 7.1 and this is most probaly happening because of a one of required PHP extension is not enabled on the latest versions. I need to check what extension should be enabled/disabled on latest PHP versions.

Screenshot 2020-04-20 at 17 42 39

@hostep
Copy link
Contributor

hostep commented Apr 20, 2020

@tushhan: you need the fileinfo php extension, I already mentioned it above 😉
Good luck!

Strictly speaking, it's a bug in Magento however, there is a suggestion to fix it, but might take a while until somebody picks it up.

@antonfries
Copy link

We got this error in 2.3.7-p1 in a very big project. The fix was to increase/deactive the max_session_size in admin. The cause was, that the current_path-variable couldn't be saved persistently in the session-object for the upload to work.

@pisarkov
Copy link

pisarkov commented Sep 14, 2021

Fixed by increasing session size values (Configuration -> System -> Security).

Magento Commerce 2.4.3

image

@hafizmuzamal
Copy link

thank you @pisarkov it worked in my case

@marcelloinfoweb
Copy link

@pisarkov, It didn't work for me.

Magento 2.4.3

@hafizmuzamal
Copy link

@marcelloinfoweb what is the exact issue you are facing right now?

@marcelloinfoweb
Copy link

marcelloinfoweb commented Oct 13, 2021

@marcelloinfoweb qual é o problema exato que você está enfrentando agora?

Error_delete-image
Error_insert-image

message error in log: main.ERROR: Notice: Undefined index: force_static_path in /var/www/magento/vendor/magento/module-cms/Controller/Adminhtml/Wysiwyg/Images/OnInsert.php on line 57

@hafizmuzamal
Copy link

hafizmuzamal commented Oct 14, 2021

@marcelloinfoweb Have you set the proper permissions to your magento folders?
Please follow this link for setting the permissions:
https://devdocs.magento.com/guides/v2.4/config-guide/prod/prod_file-sys-perms.html

@focus97
Copy link

focus97 commented Dec 7, 2021

Getting same issue as @marcelloinfoweb

@marcelloinfoweb - were you able to find a fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Event: balance-cd Event: distributed-cd Distributed Contribution Day Event: kiev-cd Fixed in 2.3.x The issue has been fixed in 2.3 release line 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 Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development 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

No branches or pull requests