-
Notifications
You must be signed in to change notification settings - Fork 255
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
/Sources/ShowAttachments.php ~ compatibility fix #3382
Conversation
- SMF 2.0.X branch did not use a sytematic file extension on attachments (it was void of any extensions) - this edit will allow for any generic extension to be removed when said file does not exist in case someone decides to change the behaviour yet again Signed-off-by: -Underdog- github.underdog@gmail.com
- fixes specific usage of the array which was flagging errors for guests Signed-off-by: -Underdog- github.underdog@gmail.com
@@ -110,6 +110,7 @@ function showAttachment() | |||
$smcFunc['db_free_result']($request); | |||
|
|||
$file['filePath'] = getAttachmentFilename($file['filename'], $attachId, $file['id_folder'], false, $file['file_hash']); | |||
$file['filePath'] = !file_exists($file['filePath']) ? substr($file['filePath'], 0, -(strlen(pathinfo($file['filePath'])['extension'])+1)) : $file['filePath']; |
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.
pathinfo($file['filePath'])['extension']
This will generate a parse error on PHP < 5.5.
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 forgot about that change to PHP.
I'm glad they changed it because 2 lines of code to do one small task seems superfluous.
- PHP < 5.5 can not array dereference an array literal Signed-off-by: -Underdog- github.underdog@gmail.com
The attachments issue was a correct fix but the guest $user_info issue was not. |
- not the source of the issue seen. New pull request to be posted later. Signed-off-by: -Underdog- github.underdog@gmail.com
- array_intersect does not allow simple array vs multidimensional array comparison - array to string conversion error was contantly showing in log prior to this edit Signed-off-by: -Underdog- github.underdog@gmail.com
Btw - these issues were apparent when testing an upgrade from the SMF 2.0.X branch. |
Wouldn't it be easier if you just cast the var as an array? $permission = (array) $permission; since permissionCallback() is not going to be used anywhere else might as well use a lambda |
- reverted 1 change Signed-off-by: -Underdog- github.underdog@gmail.com
- changed standard closure to lambda as per request Signed-off-by: -Underdog- github.underdog@gmail.com
I used gettype because of something I read regarding using is_array with a single empty key of zero not flagging true but I can't seem to find the reference. Even though when I tested this last I had no issue with that scenario. |
Dunno why you reverted it, I just said it would be better to just cast the var as an array and avoid doing checks altogether. |
That changes the array to have an initial key of zero. ie. Output is:
|
Please do note that there's a difference between $var = (array) $var; That's casting as an array. And this $var = array($var) ; That's adding the var as a value in another new array. Casting an array as an array does nothing to the array, which is why I suggested it. It only modifies the var if the var is not an array. Underdog notifications@github.com wrote:
|
- ensure $permission is an array prior its comparison Signed-off-by: -Underdog- github.underdog@gmail.com
- not tabbed properly Signed-off-by: -Underdog- github.underdog@gmail.com
Oops.. my mistake. Imo I'm a bit rusty as of late and this is the most revised PR I've ever entered. |
re. more regarding behaviour of SMF 2.0.X -> SMF 2.1.X |
Avatars shouldn't be saved as attachments anymore. The upgrade is supposed to convert any existing avatars as normal images. Al so check this PR #3377 it has some changes to the way showAttachment() work |
I figured as much. |
With the recent additions this PR now has merge conflcits, do you want to resolve them or create a brand new PR based on the recently updated files? |
- adjusted file to resolve conflicts with other recent changes Signed-off-by: -Underdog- github.underdog@gmail.com
I tried to resolve the conflict but there must be more edits to the files therefore I will do another p/r as you suggested. |
closed new p/l: #3402 |
Signed-off-by: -Underdog- github.underdog@gmail.com