Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Strange side effects of 5.0.7 #17655

Open
dereuromark opened this issue Apr 6, 2024 · 4 comments
Open

Strange side effects of 5.0.7 #17655

dereuromark opened this issue Apr 6, 2024 · 4 comments
Labels
Milestone

Comments

@dereuromark
Copy link
Member

Description

https://github.com/dereuromark/CakePHP-DatabaseLog/actions/runs/8582848427/job/23521405236

1) DatabaseLog\Test\TestCase\Controller\LogsControllerTest::testDelete
Failed asserting that 4 is identical to 0.

/tests/TestCase/Controller/LogsControllerTest.php:107

Those started to fail with the release of 5.0.7 now

Nothing changed, it also only seems to happen for SQlite

From the changes I cannot directly see how this makes sense.
5.0.6...5.0.7

But it seems there is some unintended side effect here in the changes.

CakePHP Version

5.0.7

PHP Version

8.2

@dereuromark dereuromark added this to the 5.0.8 milestone Apr 6, 2024
@dereuromark
Copy link
Member Author

dereuromark commented Apr 7, 2024

Looking into what those extra logs are it seems super weird ones:

MissingTemplateException - Failed to render error template `error400`. Error: Template file `Admin/Error/error400.php` could not be found.

The following paths were searched:

- `/work/devilbox/data/www/sandbox/vendor/dereuromark/cakephp-databaselog/tests/test_app/templates/plugin/DatabaseLog/Admin/Error/error400.php`
- `/work/devilbox/data/www/sandbox/vendor/dereuromark/cakephp-databaselog/templates/Admin/Error/error400.php`
- `/work/devilbox/data/www/sandbox/vendor/dereuromark/cakephp-databaselog/tests/test_app/templates/Admin/Error/error400.php`
- `/work/devilbox/data/www/sandbox/vendor/dereuromark/cakephp-databaselog/vendor/cakephp/cakephp/templates/Admin/Error/error400.php

I did create the actual templates in tests/test_app/templates/Error/
The Admin prefix seems weird here.

@dereuromark
Copy link
Member Author

dereuromark commented Apr 7, 2024

So the issue was already there before, just not visible without the logs being written.
Now that they are these issues show up.
dereuromark/CakePHP-DatabaseLog#51 fixes it, so it begs the question why the Admin/ prefixed ones are necessary in the first place.
So far usually the templates for error logs are always without the prefix.

If we allow Admin/ ones to be looked for first, the fallback should still be the former ones (normal, without prefix) I imagine.

@markusramsak
Copy link
Contributor

markusramsak commented Apr 10, 2024

A similar weird issue exists if you use the FormProtection Component in the initialize() method.
If it throws (i.e. make a POST request with wrong data) I always get a log entry like:
Failed to construct or call startup() on the resolved controller class of App\Controller\ErrorController. Using Fallback Controller instead. Error Form tampering protection token validation failed.

public function initialize() {
      $this->loadComponent('FormProtection');
}

my error controller looks like:

declare(strict_types=1);

namespace App\Controller;

use Cake\Core\Configure;

class ErrorController extends AppController
{
    public function beforeRender($event)
    {
        $this->viewBuilder()->setTemplatePath('Error');
    }
}

I solved this by changing the ErrorController to:

declare(strict_types=1);

namespace App\Controller;

use Cake\Core\Configure;

class ErrorController extends AppController
{
    public function initialize(): void
    {
        Configure::write('isErrorController', true);
        parent::initialize();
    }

    public function beforeRender($event)
    {
        $this->viewBuilder()->setTemplatePath('Error');
    }
}

and my AppController to:

public function initialize() {
      if (!Configure::read('isErrorController')) {
          $this->loadComponent('FormProtection');
      }
}

Is there a better to solve this or is this the only solution if I want to use my custom AppController?

@markstory
Copy link
Member

Is there a better to solve this or is this the only solution if I want to use my custom AppController?

There isn't a significantly better solution. If you have FormProtectionComponent enabled it wants to do its thing on every controller action and ErrorController is a controller. I personally don't include components that will cause problems in error handling in AppController. Another solution that I prefer is

class ErrorController extends AppController
{
    public function initialize(): void
    {
        parent::initialize();
        $this->components()->unload('FormProtection')
    }

    public function beforeRender($event)
    {
        $this->viewBuilder()->setTemplatePath('Error');
    }
}

I prefer this solution as it doesn't introduce additional global state.

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

No branches or pull requests

3 participants