Trying to access array offset on value of type bool

[sah62]

I just noticed this error in my error log:

Trying to access array offset on value of type bool

This is an installation of SMF 2.1.2, default theme, UTF-8 supported, no non-English language packs, level 1 caching, no mods, PHP 7.4.3.

Backtrace:

Code Select Expand

#0: smf_error_handler()
Called from /var/www/mysite/smf/Sources/Subs-Attachments.php on line 1251
#1: loadAttachmentContext()
Called from /var/www/mysite/smf/Sources/Subs-Attachments.php on line 1048
#2: parseAttachBBC()
Called from /var/www/mysite/smf/Sources/Subs.php on line 1746
#3: {closure}()
Called from /var/www/mysite/smf/Sources/Subs.php on line 3636
#4: parse_bbc()
Called from /var/www/mysite/smf/Sources/Display.php on line 1485
#5: prepareDisplayContext()
Called from /var/www/mysite/smf/Themes/default/Display.template.php on line 256
#6: template_main()
Called from /var/www/mysite/smf/Sources/Load.php on line 2747
#7: loadSubTemplate()
Called from /var/www/mysite/smf/Sources/Subs.php on line 4155
#8: obExit()
Called from /var/www/mysite/smf/index.php on line 194

The code involved:

Code Select Expand

1245: // Calculate the size of the created thumbnail.
1246: $size = @getimagesize($filename . '_thumb');
1247: list ($attachment['thumb_width'], $attachment['thumb_height']) = $size;
1248: $thumb_size = filesize($filename . '_thumb');
1249:
1250: // What about the extension?
==>1251: $thumb_ext = isset($context['valid_image_types'][$size[2]]) ? $context['valid_image_types'][$size[2]] : '';

The issue is that getimagesize can return boolean false on failure. When that happens, $size isn't an array and $size[2] will produce an error. The code needs to check the return value from getimagesize and do something appropriate if it's boolean false.

[SpacePhoenix]

The error suppression operator ( the @ ) has itself been suppressed so that all errors are displayed/logged

xdebug can be set so the the error suppression operator is ignored and the relevant error still gets displayed.

A question in StackOverflow covers it:

https://stackoverflow.com/questions/55928459/how-do-i-disable-error-suppression-operator-in-php

imo the error suppression operator should have been removed from PHP

[Arantor]

The @ has legitimate (if historical) use where you cannot possibly know in advance if an operation will fail until you try it - but failure guarantees an error message because proper error handling is still a mystery to PHP's devs in places.

For example, writing to a file, you literally cannot know before you try writing the file that you will *succeed* in writing the file. Because even if you do is_writable() beforehand, that doesn't verify that a) in the time between the is_writable() call and the write call that something hasn't changed the situation, especially on network shared drives, or even that b) there is sufficient space to write your file anyway.

Now, it's entirely possible to handle these failure states meaningfully after the fact but this is the sort of thing exceptions deal with, except that almost all of the file handling functions predate exception support, and changing them now breaks things because that's not how you do backward compatibility handling.

In this case though this isn't actually just a 2.1 bug, it's actually present in 2.0 as well if the topic display code has to regenerate the thumbnail but for whatever reason it's not actually able to (e.g. uploading a PSD)

[live627]

The error suppression operator ( the @ ) has itself been suppressed so that all errors are displayed/logged

where, on your server?

[SpacePhoenix]

The error suppression operator ( the @ ) has itself been suppressed so that all errors are displayed/logged

where, on your server?

I use XDebug on my WAMP server and have got it configured so that it suppresses the error suppression operator

[Kindred]

So, how is your non-standard configuration a bug in the software?

[Arantor]

The bug is that the assumption is getimagesize() will have been fed an image it can work with and returned an array, but it won't always return an array. The @ is a distraction beause getimagesize() can also emit an error if given something that's not an image.

In this case the @ should even stay because you don't *know* that the thing is actually an image, but once the getimagesize call has been made, you can know to check it before the list() call.

[SpacePhoenix]

is_array() could be used on $size before the list() call with an appropriate error or exception generated if it's not an array

[Arantor]

But you know you still need to leave the @ there, right?

[SpacePhoenix]

Something has changed somewhere, as I downloaded and installed a clean copy, been messing around with it a very tiny amount.

Done a test attachment to a post which worked ok, and yet Subs-Attachments.php does not exist. Looks like ManageAttatchments.php has taken over its role

[Arantor]

That sounds like you're looking at 2.0 rather than 2.1.

The offending piece of code is in Display.php in 2.0 (since it's rebuild-thumbnail-in-thread), but in Subs-Attachments.php in 2.1 as per the OP.

[SpacePhoenix]

Still no error with 2.1. Both images that are loo large (both file size and dimensions) or invalid file types. Tried for both attachments and avatar upload. Also invalid file types.

Both PHP 7.3 and 7.4

Still with the suppression or the error suppression operator

[Arantor]

How are you testing, exactly? If you're doing this by way of uploading attachments, that's not enough.

Specifically the chunk of code is triggered by coming to the topic when an attachment that should have a thumbnail doesn't have a thumbnail - but the thing is also not able to be handled.

The sort of case I'm getting at is that you'd have to be at the topic itself, looking at an uploaded file (by way of the attach bbc), something that claimed to be an image - maybe even was an image originally - but for whatever reason fails in getimagesize. And that didn't have a thumbnail at the point of visiting the topic.

It's pretty specific to set up and test for.

[SpacePhoenix]

Tried uploading a file that's too big (image file). It uploads, no thumbnail. When clicked on gives File Not Found as a plain screen (no themes etc loaded). Still no PHP error.

[Arantor]

It's not the "too big" path that's the issue - it'll still have a valid size. You need to get into the workflow of "this is a file that to all intents and purposes should have a thumbnail" but when it actually comes time to make it, *it is not a valid image*. Too big doesn't make it not an image.

[SpacePhoenix]

Still not able to replicate it, any file that should generate a thumbnail still generates a thumbnail. Maybe the OP could upload a file that's known to causes the suspected issue

[Arantor]

We've actually spent more time debating if this is an issue than it would actually take to fix it.

[sah62]

Still not able to replicate it, any file that should generate a thumbnail still generates a thumbnail. Maybe the OP could upload a file that's known to causes the suspected issue

I don't know what my forum user was doing to cause the issue. I just saw the error in my log. We know what the problem is, though: getimagesize can return boolean false, and when that happens "false" isn't an array. The easiest thing to do is to test the value of $size returned from getimagesize and do whatever is appropriate if it isn't an array. Perhaps something like this:

Code Select Expand

$size = @getimagesize($filename . '_thumb');
if (is_bool($size)) {
  // Do whatever is appropriate in the error situation.
}
else {
  list ($attachment['thumb_width'], $attachment['thumb_height']) = $size;
  $thumb_size = filesize($filename . '_thumb');

  // What about the extension?
  $thumb_ext = isset($context['valid_image_types'][$size[2]]) ? $context['valid_image_types'][$size[2]] : '';
  // ...and whatever is appropriate if there's no error.
}


[sah62]

Sorry, I went back to my error log and I noticed that the log includes the topic that my forum user was viewing. It contains three attachments. Unfortunately, when I viewed the topic all three attachments were properly displayed as thumbnails and no error was produced.

[Steve]

Is the issue resolved then?

[Arantor]

No, it's a legitimate bug, albeit one that can't readily be reproduced.