bramus / mixed-content-scan

Scan your HTTPS-enabled website for Mixed Content
MIT License
522 stars 51 forks source link

Use PSR-3 LoggerInterface in Scanner #52

Closed GaryJones closed 7 years ago

GaryJones commented 7 years ago

While line formatting aspects of Monolog are defined in the binary file, the scanner can be adapted to accept any logger (including Monolog) that is compatible with PSR-3.

Maybe PSR-3 LogLevels could be used as part of the binary CLI Options (default is currently a magic number of 200), but that is outside the scope of this PR.

bramus commented 7 years ago

Hi @GaryJones,

Which version of Monolog are you using? I'm asking this because in the 1.x branch of Monolog the methods are named addInfo,addDebug, etc.. It's only on master that the new naming conventions are used

Currently mixed-content-scan is locked down to 1.19 (which follows the “old” naming conventions). An updated composer.json/composer.lock seems to missing from this commit to make this all work.

Regards, Bramus.

GaryJones commented 7 years ago

My composer.lock shows 1.19.0, as you said. However, according to the Monolog Readme:

As of 1.11.0 Monolog public APIs will also accept PSR-3 log levels.

If you look further down from your first link in the 1.x branch, you'll see the PSR-3 named methods:

Your link: https://github.com/Seldaek/monolog/blob/1.x/src/Monolog/Logger.php#L354 PSR-3 methods: https://github.com/Seldaek/monolog/blob/1.x/src/Monolog/Logger.php#L530

This commit added the PSR-3 levels back in Sept 2014 and it shows as being included in the 0.11.0 tag.

The current composer.json has a Monolog requirement of ~1.0, so there doesn't appear to be a specific minor version of Monolog that MCS is relying on. As such, a separate commit which does a composer update (as it's not a requirement for this PR) would probably make sense so that it references the latest (1.22.0) version in composer.lock (or at least investigate why that won't work without changes in it's own ticket scope). The changes between 1.19 and 1.22 include some handy PHP 7.0 / 7.1 fixes amongst other bits. I'm running PHP 7.1.0 locally, and whilst I've not found any problems with MCS, it probably makes sense to use a version of Monolog that has better compatibility too.

I'll update the conflict here.

GaryJones commented 7 years ago

Conflict fixed.