Logs, Logs, Wherefore Art Thou Logs?
The recently released SUPEE-10415 patch introduced some changes relating to validation of the log file paths before writing to them (APPSEC-1913). There is one side affect of these changes that I’ve not seen anybody talking about so I thought I’d write this post.
The consequence I’m refering to is caused by this change, in
- $file = empty($file) ? 'system.log' : $file;
+ $file = empty($file) ? 'system.log' : basename($file);
It is fairly common for 3rd party extensions to try to ‘namespace’ their log files by putting them in sub-directories. This is normally achieved by doing something along the lines of:
Mage::log('some log data', null, 'my_ext/general.log');
The introduction of
basename in the log method results in the
my_ext/ part being stripped off and thus the log entry
some log data is written to
As an isolated change this is not a big issue, though it will likely cause some confusion as to why log files have seemingly ‘stopped logging’, when in reality they have just written to a different filepath than expected. There are, however, some things that need to be considered from a custom code / infrastructure standpoint:
- If you use generic filenames, your log data will be merged with the log data of other extensions.
- Custom functionality you have for displaying your log data in the admin panel*, will likely read the content from the wrong file, thus not show the desired information.
- Log rotate may be either not rotating the files any more or using different configuration settings to before.
- Clean-up crons / actions for removing old log data may no longer remove the correct file.
- Attempts to parse data from your log files will no longer use the right path.
* From my experience this kind of functionality is often vulnerable to ‘stored XSS’, please make sure you’re escaping any log data before outputting it to the page. Even if it’s currently impossible to manipulate the logged data, some future update may change that and you’re unlikely to remember to come back and fix this.
In the long term, to avoid these issues, it likely makes sense to namespace your filenames, rather than through sub-directories. This would make our example file
As a final note to 3rd party developers; The patch also introduced limitations on the file extensions that can be used. So for example, if you’re logging JSON data for consumption by a log reader, you’ll want to use
.log still rather than