-
Notifications
You must be signed in to change notification settings - Fork 19
fix Notice: Undefined index: 0777 in Stat->mergeMeta #19
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
base: master
Are you sure you want to change the base?
fix Notice: Undefined index: 0777 in Stat->mergeMeta #19
Conversation
…ergeMeta() (line 157 of \vendor\twistor\flysystem-stream-wrapper\src\Flysystem\Plugin\Stat.php).
The appveyor failed on chocolatey windows package manager. This looks like it should fix |
Hey, |
@twistor I too would love to see this or a similar fix merged and released, as it should fix PHP 7.3/7.4 compatibility. |
+1 |
@twistor ping |
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.
Thanks for this improvement! This change fixes the problem for my current project.
I made some suggestions: one little tweak to the code and several changes to be consistent with the commenting standards used in this project.
/** | ||
* Normalize a permissions string. | ||
* | ||
* @param string $permissions |
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.
* @param string $permissions | |
* @param string $permissions A permissions string, such as '644' or | |
* 'drw-rw-r--' |
* | ||
* @param string $permissions | ||
* | ||
* @return int |
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.
* @return int | |
* @return int The numeric version of the permissions. |
return $permissions & 0777; | ||
} | ||
|
||
// remove the type identifier |
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.
// remove the type identifier | |
// Remove the type identifier. |
// remove the type identifier | ||
$permissions = substr($permissions, 1); | ||
|
||
// map the string rights to the numeric counterparts |
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.
// map the string rights to the numeric counterparts | |
// Map the string rights to the numeric counterparts. |
$map = ['-' => '0', 'r' => '4', 'w' => '2', 'x' => '1']; | ||
$permissions = strtr($permissions, $map); | ||
|
||
// split up the permission groups |
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.
// split up the permission groups | |
// Split up the permission groups. |
// split up the permission groups | ||
$parts = str_split($permissions, 3); | ||
|
||
// convert the groups |
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.
// convert the groups | |
// Convert the groups. |
return array_sum(str_split($part)); | ||
}; | ||
|
||
// converts to decimal number |
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.
// converts to decimal number | |
// Converts to decimal number. |
} | ||
|
||
// remove the type identifier | ||
$permissions = substr($permissions, 1); |
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.
$permissions = substr($permissions, 1); | |
$permissions = substr($permissions, -9); |
This will work with or without the initial d
or -
for regular file vs. directory. I think this also makes it clear that we expect 9 characters.
I considered padding with -
characters to ensure 9 characters, but I am not sure when that would be likely to make a difference. Pad on the left or the right? Maybe it is a bad idea to fix problems that no one has.
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 feel like the original code is closer to what the comment says it's trying to do, but indeed -9
is clearer for the actual intent here: extracting the permission groups in isolation.
Padding would make it mildly safer to use with the code below it, and if you're using substr
from the right then I'd say it makes sense to pad the left, if at all.
$permissions = substr($permissions, 1); | |
$permissions = substr(str_pad($permissions, 9, '-', STR_PAD_LEFT), -9); |
The comment could be updated to:
// Extract the permission groups.
The trickiest thing is how vague the file metadata is with Flysystem, but I suppose it has to be (for v1.x at least 😉).
fix undefined index in mergeMeta
Notice: Undefined index: 0777 in Twistor\Flysystem\Plugin\Stat->mergeMeta() (line 157 of \vendor\twistor\flysystem-stream-wrapper\src\Flysystem\Plugin\Stat.php)
using flysystem 1.0.63 and "Local" adapter
see #12
see #15