Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

excluded_404s does not work for symfony 3 #166

Closed
zerkms opened this issue May 4, 2016 · 22 comments
Closed

excluded_404s does not work for symfony 3 #166

zerkms opened this issue May 4, 2016 · 22 comments
Milestone

Comments

@zerkms
Copy link
Contributor

zerkms commented May 4, 2016

The generated NotFoundActivationStrategy service in a service locator looks like:

    protected function getMonolog_Handler_Errors_NotFoundStrategyService()
    {
        $this->services['monolog.handler.errors.not_found_strategy'] = $instance = new \Symfony\Bundle\MonologBundle\NotFoundActivationStrategy(array(0 => '^/'), 400);

        $instance->setRequest(NULL);

        return $instance;
    }

So the request is never passed into the object and it does not take the passed url regexes into account.

@zerkms
Copy link
Contributor Author

zerkms commented May 4, 2016

So, seems like there is a new implementation there already https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Monolog/Handler/FingersCrossed/NotFoundActivationStrategy.php but not sure if it's possible to switch to it so that it also worked for v2.3

@Seldaek Seldaek added this to the 3.0 milestone May 20, 2016
@Seldaek
Copy link
Member

Seldaek commented May 20, 2016

This is related to #94

@BPScott
Copy link
Contributor

BPScott commented May 25, 2016

Found that this was biting me today, so I've done a bit of digging (this is likely to be obvious to the maintainers but hey why not):

excluded_404s no longer works in Symfony 3.0 as currently monolog-bundle injects a reference to request from the service container into the NotFoundActivationStrategy. Having Request in the service container was deprecated in Symfony 2.7 in favor of using RequestStack, and subsequently removed in Symfony 3.0. As the NotFoundActivationStrategy never gets a request object it can never check the path of that request and thus the log line is never excluded.

The concept of RequestStack was added in Symfony 2.4.
The implementation of NotFoundActivationStrategy in monolog-bridge uses RequestStack and was added as part of Symfony 2.6.

#94 talks about maintaining compatibility with 2.3, but given that 2.3 entered the security fixes only part of it's LTS lifecycle this month (May 2016) I don't think the extra effort to support it is worthwhile.

From what I can tell, #94 needs to be updated to pass the RequestStack into the NotFoundActivationStrategy's constructor here then it can be merged and a release can be tagged that supports Symfony 2.6+. @Seldaek / @fabpot I can give that a go at some point soonish if you're busy with other stuff.

@BPScott
Copy link
Contributor

BPScott commented May 25, 2016

Oh, and moving away from using request shall also render #123 invalid as we won't be using a synthetic service.

@rommsen
Copy link
Contributor

rommsen commented Jul 20, 2016

Is there any plan to include a solution for this in an upcoming release? We are using the fingers crossed handler to get notified via hipchat when one of our system throws an Exception. Since moving to Symfony 3 we are bombarded with 404 notifications from bots trying to access wp_admin.php admin.php etc. which renders the notifications almost useless.

Is there any other way to mitigate this?

@zerkms
Copy link
Contributor Author

zerkms commented Jul 20, 2016

@rommsen

you can create a custom activation strategy. This is how I implemented it for my projects

use Monolog\Handler\FingersCrossed\ActivationStrategyInterface;
use Monolog\Handler\FingersCrossed\ErrorLevelActivationStrategy;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

class NotFound implements ActivationStrategyInterface
{
    private $errorLevelActivationStrategy;

    public function __construct($actionLevel)
    {
        $this->errorLevelActivationStrategy = new ErrorLevelActivationStrategy($actionLevel);
    }

    public function isHandlerActivated(array $record)
    {
        if (isset($record['context']['exception'])) {
            if ($record['context']['exception'] instanceof NotFoundHttpException) {
                return false;
            }
        }

        return $this->errorLevelActivationStrategy->isHandlerActivated($record);
    }
}

@rommsen
Copy link
Contributor

rommsen commented Jul 21, 2016

@zerkms Thanks a million. This led me in the right direction.

If anybody stumbles over this, here is how I solved it for us:
The implementation of NotFoundActivationStrategy in monolog-bridge uses RequestStack and was added as part of Symfony 2.6.

So I added a service definition for this class.

custom_monolog_fingers_crossed_handler_activation_strategy:
  class: Symfony\Bridge\Monolog\Handler\FingersCrossed\NotFoundActivationStrategy
  arguments:
    - "@request_stack"
    - ["^/"]
    - "ERROR"

First parameter is the RequestStack, second parameter are the "excluded_404s" or blacklist, third parameter is the action_level, when the fingers_crossed handler is activated (we use ERROR in this case but it defaults to WARNING in monolog-bundle).

Then I added this service as the activation_strategy parameter in my monolog config:

monolog:
  ...
  handlers:
     ...
          type: fingers_crossed
          ...
          activation_strategy: custom_monolog_fingers_crossed_handler_activation_strategy

Important: I had to remove the "excluded_404s" parameter, because it is not allowed in combination with a custom activation_strategy.

Hope this helps someone.

@LaurentMarquet
Copy link

@rommsen I am facing the same problem (as everyone using this functionality should). Do you think that I should implement your solution or wait that an official correction comes ?

@rommsen
Copy link
Contributor

rommsen commented Sep 6, 2016

@Laurent3170 I think this depends on how much pain you are feeling right now 😄 . For us it was just way too much noise so I implemented it and never looked back. In case if something more "official" arrives, it would not be to much work to revert.

@LaurentMarquet
Copy link

@rommsen just a small question how do you separate multiple excluded in the second argument ? i.e

- ^/admin.php
- ^/administrator
- ^/configuration.php
- ^/index.php

@zerkms
Copy link
Contributor Author

zerkms commented Sep 8, 2016

@Laurent3170

I believe

- ["^/admin.php", "^/administrator", ...]

would work

@LaurentMarquet
Copy link

Ok, I was guessing between "," and "|" but I can't make it work (for the time being) with just only one...
Thanks for your answer

@LaurentMarquet
Copy link

LaurentMarquet commented Sep 8, 2016

Yes that's "," I had to clear the cache... Thanks
@zerkms really thanks for your answer, I should have test before asking... A kind of lazy time :-(

vierbergenlars added a commit to IndustriaLeuven/WebV1-authserver that referenced this issue Sep 27, 2016
Using the exclude_404s configuration parameter does not work in symfony 3. (See symfony/monolog-bundle#166 for details)
@fabpot fabpot closed this as completed in d8b3f8a Oct 19, 2016
@LaurentMarquet
Copy link

LaurentMarquet commented Oct 19, 2016

As I'm quite new to Symfony, can you explain what it means ? Can we revert to what is explained in the Symfony docs ? Thanks

@StevendeVries
Copy link

@Laurent3170 with the mentioned fix by fabpot the 404s (NotFoundActivationStrategy) should work as documented for symfony 3.0.

@LaurentMarquet
Copy link

Argh, thanks for your answer but it seems not...
I am using monolog/monolog 1.21.0 and symfony/monolog-bundle 2.12.0 along with symfony 3.1.6

@StevendeVries
Copy link

@Laurent3170 try monolog-bundle 3.0: https://github.com/symfony/monolog-bundle/releases/tag/3.0.0

@LaurentMarquet
Copy link

@StevendeVries Thanks again for your prompt answer. I haven't seen that a 3.0 version was available as I was looking in the "branches" part and not in the "tags" one. So... It works! 😄

@LaurentMarquet
Copy link

Don't know where to "ask" and it seems, here it may be a place of concerns. As those servers which bother us by asking pages using specific urls, i.e. /admin, I was wondering of not setting them in config_prod.yml to avoid receiving monolog notifications but to build a specific Controller to redirect them to somewhere...
I know I can do it by myself, but maybe there are things I haven't thought about and maybe you know a good place to send them to...

@rommsen
Copy link
Contributor

rommsen commented Dec 15, 2016

maybe you know a good place to send them to...

to hell maybe? 😄 (scnr)

@thelem
Copy link

thelem commented Dec 20, 2016

I'm not really familiar with the Symfony documentation standards, but this seems like it should be mentioned on https://symfony.com/doc/current/logging/monolog_regex_based_excludes.html (at least for Symfony 3.0 and 3.1, perhaps for later versions too). Or maybe bump Symfony's dependency to ensure people don't see the problem on later versions?

@xabbuh
Copy link
Member

xabbuh commented Dec 21, 2016

@thelem Would you mind opening an issue in the docs repository?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants