Fix spl_object_hash deprecation on PHP 8.6 - #254
Conversation
e0fa0b8 to
52920f9
Compare
| $uri = $this->createUri($response, $request); | ||
| $redirectRequest = $this->buildRedirectRequest($request, $uri, $statusCode); | ||
| $chainIdentifier = spl_object_hash((object) $first); | ||
| $chainIdentifier = \PHP_VERSION_ID < 70200 ? spl_object_hash((object) $first) : (string) spl_object_id((object) $first); |
There was a problem hiding this comment.
I am surprised this ever worked. According to docs:
This id can be used as a hash key for storing objects, or for identifying an object, as long as the object is not destroyed. Once the object is destroyed, its hash may be reused for other objects.
And indeed, running:
var_dump(spl_object_hash((object) ['a', 'b']));
var_dump(spl_object_hash((object) ['a', 'b', 'c']));
var_dump(spl_object_hash((object) md5(...)));Prints the following for me:
string(32) "00000000000000020000000000000000"
string(32) "00000000000000020000000000000000"
string(32) "00000000000000020000000000000000"
There was a problem hiding this comment.
i think we only care about the identity within the plugin chain, to detect cycles. until the plugin chain is done, the $first will not be lost.
if my understanding is incorrect, then spl_object_id has the same problem:
This function returns a unique identifier for the object. The object id is unique for the lifetime of the object. Once the object is destroyed, its id may be reused for other objects. This behavior is similar to spl_object_hash().
we can merge anyways, i agree with the change and its certainly not introducing any new bug.
There was a problem hiding this comment.
Yes, not a regression. Just pointing out a weird think I noticed. If $first is an array or string, which are valid callables, as fair as I can see, they will be converted to object, which will be destroyed immediately after obtaining its id.
There was a problem hiding this comment.
ah, yep that indeed could be a problem. do you have an idea how we can avoid circular dependencies when $first is an array or string?
if we md5 the string (resp serialize the array for md5), i don't think we have the same semantics. it could make sense for an application to send the same request twice...
maybe we could clean up circularDetection when we are finished. if we can do that, it would have the upside that we don't accumulate memory in the circularDetection variable in some long-running process.
There was a problem hiding this comment.
I have not had time to dig into it deeper. Maybe we should move this into an issue for now.
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
thanks for looking into this, seems the right call.
| $uri = $this->createUri($response, $request); | ||
| $redirectRequest = $this->buildRedirectRequest($request, $uri, $statusCode); | ||
| $chainIdentifier = spl_object_hash((object) $first); | ||
| $chainIdentifier = \PHP_VERSION_ID < 70200 ? spl_object_hash((object) $first) : (string) spl_object_id((object) $first); |
There was a problem hiding this comment.
i think we only care about the identity within the plugin chain, to detect cycles. until the plugin chain is done, the $first will not be lost.
if my understanding is incorrect, then spl_object_id has the same problem:
This function returns a unique identifier for the object. The object id is unique for the lifetime of the object. Once the object is destroyed, its id may be reused for other objects. This behavior is similar to spl_object_hash().
we can merge anyways, i agree with the change and its certainly not introducing any new bug.
|
i fixed the build in #255 allowing guzzlehttp/psr7 2 and 3 made it so that all builds are green even if we do not make security exemptions. can you please rebase this branch on 2.x and remove that excemption from the composer file? if all is still green, i prefer to not have that in the configuration. |
When testing with PHP 8.6.0beta2, I get the following deprecation warning:
Function spl_object_hash() is deprecated since 8.6, consider using spl_object_id() instead
Let’s switch to the suggested function where supported.
https://www.php.net/manual/en/function.spl-object-hash.php
https://www.php.net/manual/en/function.spl-object-id.php
52920f9 to
c03f77f
Compare
|
Hmm, looks like the version of Composer used on PHP 7.1 does not support advisories and on newer versions, a more recent version will be used. Rebased. |
2 participants
What's in this PR?
Switch to the suggested non-deprecated function where supported.
https://www.php.net/manual/en/function.spl-object-hash.php
https://www.php.net/manual/en/function.spl-object-id.php
Why?
When testing with PHP 8.6.0beta2, I get the following deprecation warning:
Example Usage
N/A
Checklist