24

After this new update (1.9.4.1), Mage::log() is not working. Apparently, it has something to do with Zend_Validate_File_Extension on line 819 at Mage.php where it checks if the file is_readable() before it even exists. I reversed the whole log() method to its previous version and it is working again.

What is the main channel that i can contact Magento team to report this issue?

Improve this question
10
  • 1
    @PiotrSiejczuk That doesn't work for new log files. Your second comment implies that the ability to change log rotation configuration makes this not a serious issue and I completely disagree. Your first comment implies that this is probably only a problem for OP, or in some sort of edge case, and I very much disagree with that, too. I completely understand why Magento would not have noticed this bug, but these implications are the opposite of what is needed here (whether they are deliberate or not).
    – toon81
    Apr 1, 2019 at 16:09
  • 3
    There are many situations where this is problematic: clean installs (in this case the system.log does not yet exist), creation/installation of local and third-party modules that log to custom log files, logrotate configurations that do not explicitly create/keep the original log file.
    – Aad Mathijssen
    Apr 1, 2019 at 19:00
  • 3
    Yeah, logging is essential to every software, I wonder why they've let that pass. My dream is that when 2020 comes and Magento team stop supporting 1.x, they upload their last version to an official Git repo so that the community can keep it up to date
    – rrmesquita
    Apr 2, 2019 at 7:23
  • 1
    @cslogic "My dream is that when 2020 comes and Magento team stop supporting 1.x, they upload their last version to an official Git repo so that the community can keep it up to date" => Already done with OpenMage LTS : github.com/OpenMage/magento-lts
    – Frédéric MARTINEZ
    Apr 2, 2019 at 13:20
  • 1
    @FrédéricMARTINEZ thats funny, Ben Marks actually told me about that repo about 30 min before I read you comment.. Thanks anyway, will take a look at it
    – rrmesquita
    Apr 2, 2019 at 20:06

6 Answers 6

Reset to default
7

Official patch incoming :) Still waiting for official patch ... :(

piotrekkaminski commented 13 hours ago

This is the current official patch that will be ported to earlier versions (this should work on latest) https://gist.github.com/piotrekkaminski/0596cae2d25bf467edbd3d3f03ab9f8f

Source: https://github.com/OpenMage/magento-lts/pull/648#issuecomment-480941871

Improve this answer
7

I'll summarize everything I found so far based on research and interaction with Magento both support and Slack in regards to patching with SUPEE-11086. What can be done:

UPDATE 2: The issue is resolved in the next PATCH SUPEE-11155 - https://magento.com/security/patches/supee-11155. As always before applying the patch check for the possible issues thread - Security Patch SUPEE-11155 - Possible issues? Thanks goes to Aad Mathijssen for the great comment.

Update: An official patch is available on demand for EE version. Basically, it's Piotr Kaminski's gist wrapped as Magento patch file.

  1. Delete changes for app/Mage.php in the patch file. This is what I have done so far.
    Pros - logging works as before.
    Cons - editing a patch file, logging is unprotected from a possible exploit (but this should be very low risk). When Magento releases an official fix you will have to revert it and apply original unedited Patch.
  2. Add another patch on top based on Piotr Kaminski's gist - https://gist.github.com/piotrekkaminski/0596cae2d25bf467edbd3d3f03ab9f8f. Piotr Kaminski is part of Magento in charge of security, so this comes straight from the horse's mouth. Gist was shared in Magento slack and will probably end as SUPEE-11086 v1.1.
    Pros - This is the Magento way
    Cons - You will have to wait for this to become official, or take responsibility and package it as patch yourself, which will bring you back to having to revert once an official patch is up.
    A slight variation would be instead of adding two patches to edit the original one with those changes.
  3. Edit Zend_Validate_File_Extension::isValid and remove file existence validation. there is a long discussion in Magento LTS github - https://github.com/OpenMage/magento-lts/pull/648. The isValid method does things it's not expected to do, so some members propose to fix it. My opinion is that this is not a good solution, yes code is bad, but it was there forever and may be used in custom modules/code. On the contrary, the worst that can happen is that files are not checked for existence.
    Pros - a rather simple fix
    Cons - changes a library file and amends its functionality.
    You can apply this either as a custom patch or by rewriting the whole class in local code pool.

I chose to edit the patch, and when a v1.1 comes I'll revert the edited patch, and apply original version and after that fix. This suits well our build process and internal policy, it might be different for you. No matter what you chose it's better to apply this patch sooner than later.

Improve this answer
1
  • 1
    As of the June 25, Magento has released SUPEE-11155, Magento Commerce 1.14.4.2 and Magento Open Source 1.9.4.2 that include a fix for this issue. Essentially, the patch by Piotr Kaminski has been included.
    – Aad Mathijssen
    Jun 26, 2019 at 14:30
4

Something from Community Inputs. There is a new Validator is used Zend_Validate_File_Extension as per below:

https://github.com/brentwpeterson/magento-patches/blob/master/CE1.9/PATCH_SUPEE-11086_CE_1.9.4.0_v1-2019-03-26-03-05-04.sh#L183

"The solution is editing the patch and just removing the changes from app/Mage.php I would strongly discourage this practice, but the situation is critical".

Improve this answer
4
  • It is indeed a bad practice, but I can't survive without logging. Hope Adobe can fix it soon
    – rrmesquita
    Apr 2, 2019 at 20:08
  • yeah i had to recreate all the log files because logrotator script was deleting files and creating zips of them. I will need to find a better script of magento doesnt fix thi.
    – Kalvin Klien
    Apr 3, 2019 at 18:33
  • 1
    @KalvinKlien: Did you try with: copytruncate option of logrotate? "Truncate the original log file to zero size in place after creating a copy, instead of moving the old log file and optionally creating a new one. It can be used when some program cannot be told to close its logfile and thus might continue writing (appending) to the previous log file forever. Note that there is a very small time slice between copying the file and truncating it, so some logging data might be lost. When this option is used, the create option will have no effect, as the old log file stays in place".
    – Piotr Siejczuk
    Apr 4, 2019 at 11:32
  • Thanks @PiotrSiejczuk! I used this: /path/var/log/*log { rotate 7 copytruncate daily compress missingok notifempty }
    – Kalvin Klien
    Apr 8, 2019 at 17:33
1

My temporary solution was to copy lib/Zend/Validate/File/Extension.php to app/code/local/Zend/Validate/File/Extension.php and remove this part of the code from isValid() method:

    // Is file readable ?
    #require_once 'Zend/Loader.php';
    if (!Zend_Loader::isReadable($value)) {
        return $this->_throw($file, self::NOT_FOUND);
    }

It would become...

public function isValid($value, $file = null)
{
    if ($file !== null) {
        $info['extension'] = substr($file['name'], strrpos($file['name'], '.') + 1);
    } else {
        $info = pathinfo($value);
...

When Magento 1.9.4.2 is released I check that again.

In fact, the file not being readable, or doesn't exist, doesn't mean that the file name is not valid, right?

Improve this answer
1

I suggest to do not change core code and use an update like this (https://gist.github.com/mehdichaouch/99c67298b5a65f81219c9b69942b6fe7)

$installer->run("
    INSERT INTO `{$installer->getTable('core_config_data')}` (scope, scope_id, path, value)
    VALUES ('default', 0, 'dev/log/allowedFileExtensions', 'log,txt,html,csv')
    ON DUPLICATE KEY UPDATE value = 'log,txt,html,csv';
");
Improve this answer
0

There is another issue (which may be deliberate from the Magento team) which prevents the ability to write log files inside subfolders. For example:

Mage::log('Some log information', Zend_Log::DEBUG, 'somefolder/anotherfolder/somelogfile.log', true);

In earlier versions, that call would have created a file at location:

/your-magento-app-root-folder/var/log/somefolder/anotherfolder/somelogfile.log

But since there is a basename() function call in Mage::log() method, the file is written at:

/your-magento-app-root-folder/var/log/somelogfile.log.

Here is the incriminated code in app/Mage.php:

$file = empty($file) ?
    (string) self::getConfig()->getNode('dev/log/file', Mage_Core_Model_Store::DEFAULT_CODE) : basename($file);

Even if it is not especially related to 1.9.4.1, the issue started to happen recently (around latest 1.9.3.x versions) and is very annoying when you have to deal with a lot of log files, sometimes with the same name (but initially in different subfolders).

As that piece of code is probably deliberate from Magento team, I think there is no plan to fix it in a further release, which implies to hack it to restore the initial behavior...

Improve this answer
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.