wp_get_image_mime() - something weird going on

[jrf]

Okay, eh... help... so this is a weird one to write up.

In the context of #53009, @hellofromTonya and me are looking at all calls to markTestSkipped() in the test suite.

One of the ones I was looking at is this test in tests/phpunit/tests/functions.php:

<?php
public function test_wp_get_image_mime( $file, $expected ) {
        if ( ! is_callable( 'exif_imagetype' ) && ! function_exists( 'getimagesize' ) ) {
                $this->markTestSkipped( 'The exif PHP extension is not loaded.' );
        }

        $this->assertSame( $expected, wp_get_image_mime( $file ) );
}

for the ​`wp_get_image_mime()` function.

Now, there are two questions I have:

1. Why is is_callable() used for exif_imagetype and function_exists() for getimagesize ? This is the same in the actual function as well as the test, but strikes me as odd. Why not use function_exists() (or is_callable()) for both ?
2. I've been trying to verify the conditions by running the test with the Exif and GD extensions in various states - both enabled, GD disabled - Exif enabled, GD enabled - Exif disabled, both disabled - and I can only seem to make the test hit the skip condition with Exif disabled. Tested on both PHP 5.6 as well as PHP 7.4.

I've checked the PHP docs and the ​`getimagesize()` function is listed as part of the GD extension and the manual doesn't say anything explicitly about it being available independently of the extension, so the condition seems justified, but the fact that I can't seem to get the test to fail is making me want to dig deeper.

Related to #11946 and #40017. The original function was introduced in [39831], the tests in [40397].

Pinging @joemcgill - would you happen to be able to give me some insight into what's happening here ?

[joemcgill]

Hi @jrf! I don't know that I can remember all the details here, though I believe the reasons that these are inconsistent is that prior to [39831], WP relied entirely on getimagesize and include the function_exists() check. If I remember correctly, we introduced the exif_imagetype() check because it is more performant but wanted to keep the previous behavior as a fallback. The reason those checks are different are most likely because I personally prefer is_callable() but am also often overly conservative about changing previous behavior and opted not to update the check for getimagesize. So, the answer is probably that humans are strangely inconsistent at times 😀. Hope this helps!

[jrf]

@joemcgill Thanks for letting me know. That is one mystery solved. Now what about part 2 of my question... any insights ?

[joemcgill]

I don't think we ever fully tested the conditions where this would be skipped, to be honest, since it's so rare that these conditions would not be met. However, I would expect it to only skip when both Exif is disabled AND GD was not available. So if I'm understanding correctly and it's skipping when Exif is disabled and GD is enabled, but not when both are disabled, then something is definitely wrong here. The other thing I notice while looking at this check is that the Mark test skipped message is not really accurate, since it is not only testing for whether Exif is enabled, but is intended to check for the existence of either of the underlying dependencies.

[jrf]

@joemcgill Thanks for getting back to me about that. Basically, I could only get the test to fail when Exif was disabled. Disabling GD didn't seem to have any effect at all.

We may need to dig deeper into the PHP side of things there, but based on those tests I ran, it would appear that getimagesize() is always available, independently of GD.
If that's the case, we could most likely simplify the wp_get_image_mime() function.

[joemcgill]

That makes me really curious about what happens when you call getimagesizes() with GD disabled and if we really should improve that functionality check?

[jrf]

That makes me really curious about what happens when you call getimagesizes() with GD disabled and if we really should improve that functionality check?

@joemcgill Not sure what you mean... getimagesizes() doesn't exist as far as I can see (in WP nor PHP) and based on the tests I ran and listed above, getimagesize() just seems to work, even when GD is disabled.

Would be good to run some more tests of course, different OSes and such, but yes, if things pan out, we should be able to remove some conditions related to the availability of GD/getimagesize().

[SergeyBiryukov]

Replying to jrf:

We may need to dig deeper into the PHP side of things there, but based on those tests I ran, it would appear that getimagesize() is always available, independently of GD.
If that's the case, we could most likely simplify the wp_get_image_mime() function.

Just noting that there is indeed a ​note in the PHP documentation:

Note: This function does not require the GD image library.

[jrf]

Thanks @SergeyBiryukov !

Don't know how I missed that as I did go looking for a note of that kind (and still have the manual page open in my browser...)

Either way, in that case, sounds like we can definitely do some code simplification.

[joemcgill]

@joemcgill Not sure what you mean... getimagesizes() doesn't exist

Sorry about that, I meant getimagesize(), the s was a typo. So now I'm confused. If getimagesize() always works, then how is this test ever being skipped? Wouldn't the second condition of that if statement, ! function_exists( 'getimagesize' ) always be falsey and cause the test to run?

[jrf]

So now I'm confused.

@joemcgill Ha! We're doing a good job confusing each other 😂

If getimagesize() always works, then how is this test ever being skipped? Wouldn't the second condition of that if statement, ! function_exists( 'getimagesize' ) always be falsey and cause the test to run?

That was exactly why I opened the issue. Reading it back now, I definitely didn't explain it well enough, but yes, the test will always run.

I was looking to transform the condition to @requires tags or to remove it and with the condition as was I couldn't get the test to fail.