Repository navigation
[RedirectPlugin] potential issue in loop detection with long-running processes #256
Description
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.
Activity
Amoifr commentedon Oct 3, 2026
Hi @dbu, I'd be happy to take this one. Looking at
RedirectPlugin,$circularDetectionis 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?dbu commentedon Oct 7, 2026
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.
Amoifr commentedon Oct 9, 2026
Thanks @dbu! I went with that order in #257: the first commit only adds tests (a real two-hop circle, which already passes on 2.x, and two chains reusing the
$firstof a finished one, ending with a response or with an exception, which both fail on 2.x). The second commit clears the chain's history once it is done. The workflows are waiting for approval, so no CI has run yet. 🙂