php-http
AddPathPlugin is unreliable
PHP version: 7.4.4
Description
Our test suite is randomly failing in our integration tests since we introduced HTTPlug. This is because AddPathPlugin doesn't activate sometimes. Looks like spl_object_hash is having negative effect rather than positive. I have added some logging into your AddPathPlugin class, in form of
file_put_contents('/tmp/wat', $identifier.' = '.$request->getUri().PHP_EOL, FILE_APPEND);
before function returns. Results are
00000000386d69880000000018d3fc66 = /api/v1/product-option-values/225ec9db-aed4-4ab1-a9de-33483e4f8cd0
00000000386d781c0000000018d3fc66 = /api/v1/products/4dccedba-74e7-4223-92df-5a7050101ad5/variants/0ee7872f-b5c9-41c9-bb46-3534559562bf
00000000386d7ce40000000018d3fc66 = /api/v1/products/4dccedba-74e7-4223-92df-5a7050101ad5/variants/47864ecf-a262-4157-bc32-0b480a2dfaae
00000000386d560b0000000018d3fc66 = /api/v1/products/4dccedba-74e7-4223-92df-5a7050101ad5/variants/babfa1ad-1789-4e5b-861a-66739bc8f5f8
00000000386d5ce50000000018d3fc66 = /api/v1/products/4dccedba-74e7-4223-92df-5a7050101ad5/variants/5d37c56a-dda2-4708-ba46-c3cf9d3a6344
00000000386d4fb20000000018d3fc66 = /api/v1/products/4dccedba-74e7-4223-92df-5a7050101ad5?criteria%5Bdesigner%5D=5e9f306021c55779c10b4a12
00000000386d7f480000000018d3fc66 = /api/v1/products/4dccedba-74e7-4223-92df-5a7050101ad5?criteria%5Bdesigner%5D=5e9f306021c55779c10b4a12
00000000386d691c0000000018d3fc66 = /api/v1/channels/?criteria%5Benabled%5D=1
00000000386d69880000000018d3fc66 = /taxons/?limit=1&page=1
As you may notice, last URI is different - plugin did not do its job. This is why test failed. You can also notice that hash 00000000386d69880000000018d3fc66 is already assigned to completely different URI. This is not our fault, we create both URIs from scratch via factory, don't reuse them. PHP must be probably reusing same hashes for different objects here.
How to reproduce
Sorry, I don't have minimal reproducer. However, these tests have common one thing - using lot of Symfony\Bundle\FrameworkBundle\Client with $client->disableReboot() calls.
Possible Solution
Drop this quirky caching solution. I have tried replacing it with spl_object_id, but this is behaving same as spl_object_hash
from https://www.php.net/manual/en/function.spl-object-hash.php
This function returns a unique identifier for the object. 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.
the problem is with restarting a request, where we could be adding the path multiple times. maybe we could drop the spl id again when the request returns? as long as we are doing the request, the object won't be destroyed (even when doing async requests and going into addpath multiple times). or is there another risk involved then?
@sagikazarmark do you know how guzzle solves this problem? we might inspire ourselves from that...
Guzzle uses the RFC for URIs which says that if the input URI starts with a slash, then the original prefix is ignored, and just the host is used, and if the input has no slash, both the host and the URI prefix is used.
fixed in #211
i think the actual problem was the same problem that i recently ran into with some other library, that non-strict comparisons of spl hashes can mess up https://davidbu.ch/mann/blog/2022-09-16/esoteric-bug-php/
but i think the solution now is good enough and object hash does have the risk of clashes when memory locations are reused, which could be the case in a long running process.