-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Remove posix_getpwuid and compare only userid #11091
Conversation
Signed-off-by: Daniel Kesselberg <[email protected]>
3d03565
to
2e5d8ec
Compare
It's okay for me. At the moment the check is only for the same owner (not group). |
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.
Well, calling posix_getpwuid(
twice should not result in something different for the same input, and if it is not the same input, the result is also never the same, right?
* @param array $appRoot The app root config | ||
* @return string[] The none writable directory paths inside the app root | ||
*/ | ||
private function getAppDirsWithDifferentOwnerForAppRoot(array $currentUser, array $appRoot): array { | ||
private function getAppDirsWithDifferentOwnerForAppRoot($currentUser, array $appRoot): array { |
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.
type hint int
missing
Signed-off-by: Daniel Kesselberg <[email protected]>
Close #11088
Close #11089
posix_getpwuid returns an array with information about user/group/etc. for current user. These information is not available when /etc/passwd is not readable (see https://secure.php.net/manual/en/function.posix-getpwuid.php#45994).
@weeman1337 @rullzer is it enough to compare the user id? i guess we could check the group id as well.