Allow casting CurlHandle to int

[nikic]

It's common to cast curl resources to integer to get the resource ID. This allows casting the new CurlHandle object to int as well, returning the object ID. This seems like a simple way to improve compatibility with PHP 7.

Something to keep in mind is that unlike resource IDs, object IDs are reused.

I'm only doing this for CurlHandle, not CurlMultiHandle or CurlShareHandle. Do those need this as well?

cc @kocsismate @nicolas-grekas

[KalleZ]

Since we are changing from resources to objects, making is_resource calls into is_object and get_resource_type into instanceof, wouldn't it make sense not to do this? As the CurlHandle would be the only class to have such behavior, possibly because it is the most commonly used one, none of the other resource > object conversions do this (unless I missed it)

[nikic]

@KalleZ The other resource conversions indeed don't do this, but I would expect us to do this for other cases where it's common, e.g. if we switch streams to use objects.

is_resource() checks are not replaced with is_object() checks, they're replaced with !== false checks that work on PHP 7 and PHP 8. For the int cast, one would have to use something like

$map[\is_resource($curl) ? (int) $curl : \spl_object_id($curl)] = $curl;

instead. Which is not particularly bad of course, but also not ideal.

[kocsismate]

@nikic Thank you for the fix! I wouldn't have had the time for this now.

If we are talking about improvements related to migrated objects.. It would be nice to also lock these classes via #5572 . Or is there any problem/question that prevents you from committing it?

[nikic]

I went ahead and landed this to avoid unnecessary disruption. Happy to revert if we don't have a consensus (though per chat, it seems we do).

[GrahamCampbell]

Nice. 🎉

[derrabus]

This mechanism currently seems to work for CurlHandle instances, but not for CurlMultiHandle instances. Is this intended?

[kocsismate]

@derrabus It's written in the description:

I'm only doing this for CurlHandle, not CurlMultiHandle or CurlShareHandle. Do those need this as well?

[GrahamCampbell]

Do those need this as well?

It seems the answer is, yes. ;)

[nikic]

Can you point me to some piece of code that does this for a curl multi handle?

[derrabus]

@nikic

https://github.com/symfony/symfony/blob/2c9e9cd0c20f5206003948c63ba027ae760e060b/src/Symfony/Component/HttpClient/Response/CurlResponse.php#L60

[derrabus]

Sorry, it's rather that one here:

https://github.com/symfony/symfony/blob/2c9e9cd0c20f5206003948c63ba027ae760e060b/src/Symfony/Component/HttpClient/Response/CurlResponse.php#L253

[nikic]

@derrabus Thanks, I've applied 26171c3.

[derrabus]

@nikic Wow, thanks a lot for the quick patch. ❤️