fix(symfony,laravel): IriConverter local cache key collision between item and collection ops#7975
Open
soyuka wants to merge 2 commits into
Open
fix(symfony,laravel): IriConverter local cache key collision between item and collection ops#7975soyuka wants to merge 2 commits into
soyuka wants to merge 2 commits into
Conversation
…item and collection ops The `_c` suffix was shared by string-resource and collection-op calls, so two unrelated callsites with the same operation name and resource class returned each other's cached operation. Surfaces under worker runtimes (FrankenPHP, Swoole) where the converter persists across requests. Encode both axes independently: `_s|_o` for string vs object resource, `_c|_i` for collection vs item operation.
…item and collection ops Same collision as the Symfony converter — actually broader here, since the Laravel key didn't check `CollectionOperationInterface` at all: any string-resource call shared one slot regardless of operation type. Surfaces under Octane/worker mode where the converter persists across requests. Encode both axes independently: `_s|_o` for string vs object resource, `_c|_i` for collection vs item operation.
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.
Summary
The
IriConverter::$localOperationCachekey in both the Symfony and Laravel routing converters used a suffix scheme that collided for legitimately distinct call patterns. Under worker runtimes (FrankenPHP, Swoole, Octane) the converter survives across requests, so the collision corrupts IRI generation for any subsequent request that lands on the leaked slot.Symfony (
src/Symfony/Routing/IriConverter.php):_cwas shared byis_string(\$resource)AND\$operation instanceof CollectionOperationInterface. After a POST that triggersPurgeHttpCacheListener::postFlush, aGetCollectionoperation is cached under'' . Class . '_c'. Any later call likegetIriFromResource(Class::class, ABS_PATH, new Get(), ['uri_variables' => ['id' => 1]])hits the same slot and gets the cachedGetCollection— producing/things?id=1instead of/things/1.AbstractItemNormalizer::getResourceFromIrithen throwsInvalid IRI.Laravel (
src/Laravel/Routing/IriConverter.php): broader scope — the key didn't checkCollectionOperationInterfaceat all, so any string-resource call shared one slot regardless of operation type.PurgeHttpCacheListeneralready primes that slot with aGetCollectionon every save/delete (seesrc/Laravel/Eloquent/Listener/PurgeHttpCacheListener.php:50).Fix
Both converters now encode the two axes independently in the cache suffix:
_s/_o— string vs object resource_c/_i— collection vs item operationYielding four distinct slots (
_s_c,_s_i,_o_c,_o_i) with no overlap.Test plan
tests/Functional/Json/RelationTest::testCreateRelatedDummyWithPlainIdentifierForRelation— end-to-end Symfony repro usingRelatedDummyPlainIdentifierDenormalizer; sequentialPOST /third_levelsthenPOST /related_dummiesin the same kernel (\$alwaysBootKernel = false). Fails on4.3without the fix withInvalid IRI "/third_levels?id=1".src/Laravel/Tests/Unit/Routing/IriConverterTest::testLocalCacheKeyDistinguishesItemAndCollectionForStringResource— unit-level repro: anonymousnew GetCollection()then anonymousnew Get()for the same class-string. Fails on4.3without the fix (returns/api/booksinstead of/api/books/1).tests/Symfony/Routing/IriConverterTest(14 tests) and the broadertests/Symfony/Routing/+tests/Functional/run (691 tests) stay green.PurgerTestfailures pre-exist on4.3and are unrelated to this change (verified by stashing the fix).