Repository navigation
[RedirectPlugin] potential issue in loop detection with long-running processes #256
Description
Activity
Hi @dbu, I'd be happy to take this one. Looking at RedirectPlugin, $circularDetection is only ever appended to, so besides the id reuse it also grows for as long as the plugin lives. My idea would be to drop the chain's entry once its promise settles, with a final (non-redirect) response or with an exception, plus a test sending two unrelated requests that end up with the same chain identifier, to check the second one is not flagged as circular. Does that match the direction you had in mind?
hi @Amoifr thanks for offering. glad if you want to take a look. ideally you start with a test that reliably triggers the issue (i guess if we start fresh with the same $first instance to simulate the problem - if the request completed, and a new request is done with a $first that has the same pointer, that is not a circular redirection...
once we have the test to prove we do have the issue, and are sure there are enough tests for the cases of actual circles, lets try that approach and see if it solves the issue without regressions.
During a deprecation fix, we noticed that our loop detection might be fragile for long-running processes that trigger many requests: #254 (comment)
spl_object_id (and spl_object_hash) rely on the pointer of the object. those pointers can be reused when an object is freed. if $first is a string or array, we create an object on the fly that is immediately discarded, so the risk of reusing is higher.
we should find a solution that resets the list of visited URLs when the request is completely finished. besides avoiding false positives, this will also avoid leaking memory for no longer used history.