[2.x] fix(api): guard EloquentBuffer against orphaned/mock models#4669
Merged
Conversation
Laravel's Collection::loadAggregate fatals with "Call to a member function getAttributes() on null" when the collection contains a model whose key has no matching row in the database — the JSON:API BelongsTo linkage shortcut can produce such mock models (exists=false) for orphaned foreign keys, which then crash the request when an aggregate field (e.g. fof/upload's countRelation on UserResource) flushes the buffer. Filter non-existent models out before forwarding to load()/ loadAggregate(), and always clear the buffer entry so stale mocks don't accumulate. Refs #4667.
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
exists=false(or a null key) out of the collection passed toload()/loadAggregate()inEloquentBuffer::load(), and always clear the buffer entry so stale mocks don't accumulate.Background
Closes #4667.
Laravel's
Illuminate\Database\Eloquent\Collection::loadAggregate()(seeCollection.php:127) reads attributes off the per-key result map without a null check:If a model in the source collection has a key that doesn't match any DB row,
$models->get(...)isnulland the request fatals withCall to a member function getAttributes() on null.The JSON:API BelongsTo linkage shortcut in
EloquentResource::getRelationshipValue()(vendor/flarum/json-api-server) and the mirror in ourAbstractDatabaseResource::getRelationshipValue()create exactly this kind of mock model:When such a mock ends up in
EloquentBufferfor arelationAggregatefield (e.g. fof/upload'scountRelation('foffiles')onUserResource), the flush at the end of the request crashes the whole response with a 500 — affecting any page that serializes that resource (including the forum index for guests).Fix
In
EloquentBuffer::load():load()/loadAggregate().Test plan
EloquentBufferTest::aggregate_load_does_not_crash_for_models_that_do_not_exist_in_the_database— fails on2.xwith the exact stack from the issue, passes with this fix.tests/integration/extenders/ApiResourceTest.php— 41/41 pass.tests/integration/api/discussions/— 39/39 pass.tests/integration/api/users/+tests/integration/api/posts/— 136/136 pass.