This changes an PHPAPI function, so it breaks extensions that depend on it. What we should do in this case is to rename php_getimagetype to php_getimagetype_ex and add a "new" function called php_getimagetype which is a PHPAPI and allow php_getimagetype_ex to be called with a NULL argument as this function can be used to read from streams where a filename may not always be available.

This way we keep compatibility. We do similar things with other exposed functionality with PHPAPI, you can find some examples in ext/standard.

@KalleZ Unless there is evidence that these functions see heavy use outside of php-src, changing the signature is fine for 7.4.

Though I do wonder -- is there any way to get the file (or stream) name from the stream itself?

@nikic I don't remember if there was a reliably way, but some streams may not expose a filename like a custom stream. For ext/exif I had to implement something like:
https://github.com/php/php-src/blob/master/ext/exif/exif.c#L4255

Now I do realize the userland function getimagesizes() only supports a filename argument.

This changes an PHPAPI function, so it breaks extensions that depend on it. What we should do in this case is to rename php_getimagetype to php_getimagetype_ex and add a "new" function called php_getimagetype which is a PHPAPI and allow php_getimagetype_ex to be called with a NULL argument as this function can be used to read from streams where a filename may not always be available.

This way we keep compatibility. We do similar things with other exposed functionality with PHPAPI, you can find some examples in ext/standard.

@KalleZ, thank you for your recommendation.

It's fine to use the another function to do this without changing the php_getimagetype function because of dependency.

Should I accept this request change?

@KalleZ Unless there is evidence that these functions see heavy use outside of php-src, changing the signature is fine for 7.4.

I just found that this php_getimagetype PHPAPI is only used in exif extension at this moment.

@peter279k Can you find any other sources of usage of this function in PECL, Github or around the web just to give an idea if this is even remotely used?

Thanks for your effort btw :)

@peter279k Can you find any other sources of usage of this function in PECL, Github or around the web just to give an idea if this is even remotely used?

Thanks for your effort btw :)

H @KalleZ, thank you for your reply.

After using the php_getimagetype and image to be keywords to find related funtions usage around PECL, GitHub and web, I find them.

But they're too old or seems to be unmaintained.

The lists I found are as follows:

• https://pecl.php.net/package/imlib2
• https://pecl.php.net/package/FreeImage
• https://gynvael.coldwind.pl/?id=223

Some libraries are found in GitHub look old and inactive.

I just present the captured shots to let us know that this function is available in exif and standard image extension :).

Thank you for your research. In that case I guess its fine to change the signature for PHP-7.4, however I would still like if the filename parameter could accept NULL for the cases explained above.

PHP's Stream API does not provide a reliable way to supply a file name unless its a standard I/O operation, in that case the caller may not have a file name to supply. Something as simple as checking if filename is NULL and if so, then setting it to a value like <stream>.

What do you think? cc @nikic

Thank you for your research. In that case I guess its fine to change the signature for PHP-7.4, however I would still like if the filename parameter could accept NULL for the cases explained above.

PHP's Stream API does not provide a reliable way to supply a file name unless its a standard I/O operation, in that case the caller may not have a file name to supply. Something as simple as checking if filename is NULL and if so, then setting it to a value like <stream>.

What do you think? cc @nikic

Hi @KalleZ, thank you for your reply.

Perhaps we can let the filename be optional argument in php_getimagetype function if possible.

@KalleZ, I also add some suggested code snippets.

Are the request changes that you expect :)?

@kalle can you wrap this one up please ?

To finalize this, here would be my suggestions:

• Keep input a required parameter of php_getimagetype(). Making it optional makes printing code harder.
• Change the error message to Error reading from %s.
• For the getimagesizefromstring case make the filename "string", so the error message becomes Error reading from string.

@peter279k can we get a status update here please ?

@peter279k can we get a status update here please ?

@krakjoe, thanks for your reply. I will do latest final changes that @nikic mentions.

Thanks! Merged as 1ea329d into master, as we can't make ABI changes in 7.4 anymore.