-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(Router): Stop loading routes of disabled apps #44797
Conversation
/backport to stable29 |
f523e29
to
c3917f8
Compare
For some reason the comments integration test fails reliably because of 404 instead of 200. Seems like this patch is blocking too much? But if the dashboard app is disabled the test should have never worked. |
Let's not backport it to older versions in this case. |
Signed-off-by: provokateurin <kate@provokateurin.de>
c3917f8
to
4c5e05f
Compare
I limited the check to the part that is responsible for parsing the attributes as blocking in any case was breaking the comments integration tests (which also seems to be a bug but not in scope of the fix). |
will this be in next release 29.0.1? |
Yes, this will appear in 29.0.1 |
Hi all, What could cause this error ? |
Ok, I found my problem : I had a really old file in my files app, which was still loaded I think |
Yeah I was about to say that this file doesn't even exist anymore in the codebase 🙈 |
Thanks @philippe-ritter your comment (deleting apps/files/lib/Controller/AjaxController.php) and deleting
Fixed the issue for me. |
Thanks @philippe-ritter deleting |
Thanks a lot. Had the same issue. Solution of @philippe-ritter, deleting AjaxController.php, worked for me too. File was from 2022. |
The integrity check should tell you that already. Also it points to a wrong/problematic update strategy. |
Summary
Routes of disabled apps should not be loaded. The classes are not loaded by the autoloader and the request will fail trying to use reflection on them.
Checklist