search

inpsyde / Wonolog

Monolog-based logging package for WordPress.
https://inpsyde.github.io/Wonolog/
MIT License
173 stars 17 forks source link

DateBasedStreamHandler cannot be used when folder structure contains a space #39

Closed redelschaap closed 4 years ago

redelschaap commented 6 years ago

Version Information

Steps to Reproduce

  1. Have a WordPress installation where the folder structure contains a space (e.g. C:\WP projects\example.com)
  2. Set up Wonolog
  3. Cause an error

What I Expected

Error to be logged to wp-content/wonolog/2018-11-12.log.

What Happened Instead

Error did not get logged in wp-content/wonolog/2018-11-12.log, only in wp-content/debug.log

Debug trace

I traced this behaviour back to \Inpsyde\Wonolog\Handler\DateBasedStreamHandler::check_file_format, which is returning false because filter_var( $file_format, FILTER_SANITIZE_URL ) === $file_format is not passing.

I think this check is used to validate the filename, but I think there are other ways to do this (like with regex).

gmazzap commented 6 years ago

Hi @redelschaap

thanks for reporting.

Unfortunately regex is not an option ofr different reasons: it is quite expensive and it is not effective for file names (regex would need to be flexible enough to basically don't be useful security-wise).

I guess best option is to remove the check at all. Value is not written into database nor taken from user input, so having a relaxed check here should not be an issue.

Will look into this... but I will probably won't able to commit it this week.

redelschaap commented 6 years ago

Hi @gmazzap,

You're welcome! If a strict character check if not needed at all, I agree that removing the check will do as well.

If you want I can create a PR from a fork?

gmazzap commented 6 years ago

Hi @redelschaap

I want to be sure before removing that there's no way for user/database input to affect this. Then the check can be removed.

If you can do that, I will surely merged the PR.