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

fix(Router): Stop loading routes of disabled apps #44797

Merged
merged 1 commit into from
May 10, 2024

Conversation

provokateurin
Copy link
Member

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

@provokateurin provokateurin added bug 3. to review Waiting for reviews labels Apr 12, 2024
@provokateurin provokateurin added this to the Nextcloud 30 milestone Apr 12, 2024
@provokateurin
Copy link
Member Author

/backport to stable29

@provokateurin provokateurin changed the title refactor(Router): Remove unused variable fix(Router): Stop loading routes of disabled apps Apr 12, 2024
@provokateurin provokateurin force-pushed the fix/router/loading-disabled-apps branch from f523e29 to c3917f8 Compare April 12, 2024 09:13
@provokateurin
Copy link
Member Author

provokateurin commented Apr 12, 2024

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.

@nextcloud nextcloud deleted a comment from provokateurin Apr 12, 2024
@nextcloud nextcloud deleted a comment from provokateurin Apr 12, 2024
@nickvergessen
Copy link
Member

Let's not backport it to older versions in this case.
29 we can to after the final and patch it locally before hand to get insights

Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin provokateurin force-pushed the fix/router/loading-disabled-apps branch from c3917f8 to 4c5e05f Compare May 10, 2024 05:22
@provokateurin
Copy link
Member Author

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).

@provokateurin provokateurin merged commit 6a4e12d into master May 10, 2024
157 checks passed
@provokateurin provokateurin deleted the fix/router/loading-disabled-apps branch May 10, 2024 07:29
@BJKle
Copy link

BJKle commented May 18, 2024

will this be in next release 29.0.1?
because currently my log file gets filled with errors "OCA\UserStatus\Controller\HeartbeatController" whenever someone uses an app.
see: nextcloud/ios#2927

@provokateurin
Copy link
Member Author

Yes, this will appear in 29.0.1

@philippe-ritter
Copy link

Hi all,
even with version 29.0.1, and I checked in Router.php that I have the latest change, I got this error :
"reqId": "Zl9qwRitzrnfd2ZKakQ-gAAAA1I",
"level": 3,
"time": "2024-06-04T19:28:01+00:00",
"remoteAddr": "",
"user": "phr",
"app": "core",
"method": "GET",
"url": "/Nuage/index.php/apps/files/",
"message": "Exception thrown: ReflectionException",
"userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36",
"version": "29.0.1.1",
"exception": {
"Exception": "ReflectionException",
"Message": "Class "OCA\Files\Controller\AjaxController" does not exist",
"Code": -1,
"Trace": [
{
"file": "/home/clients/659aacf688b6ffdff44c2c628e43d209/web/famille-ritter.ch/Nuage/lib/private/Route/Router.php",
"line": 469,
"function": "__construct",
"class": "ReflectionClass",
"type": "->",
"args": [
"OCA\Files\Controller\AjaxController"
]
},

What could cause this error ?

@philippe-ritter
Copy link

Ok, I found my problem :
ls -l
total 64
-rw-r--r-- 1 uid121945 ldapusers 1624 Mar 23 2023 AjaxController.php
-rw-r--r-- 1 uid121945 ldapusers 11872 May 23 15:16 ApiController.php
-rw-r--r-- 1 uid121945 ldapusers 6353 May 23 15:16 DirectEditingController.php
-rw-r--r-- 1 uid121945 ldapusers 2018 May 23 15:16 DirectEditingViewController.php
-rw-r--r-- 1 uid121945 ldapusers 4900 May 23 15:16 OpenLocalEditorController.php
-rw-r--r-- 1 uid121945 ldapusers 3793 May 23 15:16 TemplateController.php
-rw-r--r-- 1 uid121945 ldapusers 7429 May 23 15:16 TransferOwnershipController.php
-rw-r--r-- 1 uid121945 ldapusers 13146 May 23 15:16 ViewController.php

I had a really old file in my files app, which was still loaded I think
Now after deleting it, it's working again !

@provokateurin
Copy link
Member Author

Yeah I was about to say that this file doesn't even exist anymore in the codebase 🙈

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

Successfully merging this pull request may close these issues.

None yet

5 participants