-
Notifications
You must be signed in to change notification settings - Fork 2.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
Check the return value of getUserSession before using it #30418
Conversation
Please add unit tests there |
8bdfe3f
to
c293c56
Compare
Checking if the value returned by getUserSession and getUser is null or not before using it. Signed-off-by: Sujith H <sharidasan@owncloud.com>
c293c56
to
9a1b527
Compare
Codecov Report
@@ Coverage Diff @@
## master #30418 +/- ##
============================================
+ Coverage 60.87% 60.88% +<.01%
- Complexity 18568 18572 +4
============================================
Files 1093 1093
Lines 61325 61329 +4
============================================
+ Hits 37333 37339 +6
+ Misses 23992 23990 -2
Continue to review full report at Codecov.
|
Updated with unit tests. |
Thanks. Is there also a positive test for when log conditions are set and a user is present ? |
Yes the positive tests are already there. |
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.
👍
I guess this needs to be backported? |
Creating backport PR. |
Backport PR: #30432 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Checking if the value returned by getUserSession
and getUser is null or not before using it.
Signed-off-by: Sujith H sharidasan@owncloud.com
Description
There were situations where userSession was directly calling getUser which returned null and the return value was never checked. Another case was getUserSession was returning null. Hence getUser was called on null. This caused breaking the code flow under certain conditions. This change is to fix the assumptions made.
Related Issue
#30335
#30416
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: