Repository navigation
fix(core): reset request state behind a plugin interface proxy - #681
Open
etienne-scherrer wants to merge 1 commit into
Open
etienne-scherrer wants to merge 1 commit into
etienne-scherrer wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RequestStateResetterfinds the services to clear between requests withresolvedInstances(ResettableInterface::class). When a plugin targets the interface a resettable service is bound to, the container stores the plugin's interface wrapper instead. That wrapper implements only the plugged interface andPluginInterceptedInterface, so it fails theinstanceof ResettableInterfacecheck, and the service behind it is never reset. In a long-running worker its state carries from one request into the next.A concrete subclass proxy is not affected: it extends the service, stays resettable, and holds the state itself.
Impact today
I found no first-party configuration that leaks across requests on 0.13.0:
SessionInterfacecannot be plugged: its$started { get; }property hook makes the generated wrapper fatal.GuardInterfaceresolves throughAuthManager, which resets the guards it built directly.ConnectionInterface, but its wrapper is not aTransactionInterface, so resolvingTransactionInterfacethrows before a transaction can be left open. That looks like a separate debugbar bug.It does affect an application or module that plugs the interface of its own resettable service, and #680 depends on it: there
CsrfTokenManagerInterfacebecomes a resettable singleton that remembers the token a request arrived with, and with a plugin on that interface the remembered token would stay valid for later requests on the same worker.Change
RequestStateResetterunwrapsPluginInterceptedInterfaceproxies, nested ones included, until it reaches aResettableInterface, and resets that. A concrete subclass proxy is reached first and reset as before.$exceptis matched against the service it reaches, so passinggetPluginTarget()still keeps a wrapped service alive (theRefreshDatabasecase).testing.mdsays a wrapped service is reset too.Tests
$exceptis left alone.pest --parallelinphp:8.5-cliwith the suite's extensions: 14176 passed, 449 skipped, 15 failed; the 15 (GdImageProcessorTest,GdImageProcessorLimitsTest,IntegrationVerificationTest,RepoManagementScriptsTest) fail the same way on a cleandevelopin that container.phpcs,php-cs-fixer --dry-run,docs:lintandphpstan: clean.