-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
feat(Security): Warn about using annotations instead of attributes #46606
Conversation
Signed-off-by: provokateurin <kate@provokateurin.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will spam so hard :-X
Should document similarly to nextcloud/documentation#12033 |
I already have a branch where I migrate all annotations in server (only partially done so far), does it makes sense to send a PR for that or let the app maintainers do it gradually? I fear such a big PR is not easy to review and any mistakes can be easily overlooked. |
Good medicine has to be bitter. |
I would do server in one PR yeah. |
Summary
Using annotation is a lot less safe since there is no checking that they are correctly set. For example a typo can make the annotation in-effective and thus lead to security problems. Same goes for giving arguments to the annotations, since they can not be type checked or any other type of check other than at runtime.
In contrast to that the attributes provide more safety and have been available since PHP 8.0 and thus can be used by almost every app developer out there. Adding these debug logs hopefully encourage developers to migrate to the attributes.
Checklist