Skip to content

[6.x] Prevent deleted entries from being restored - #15310

Open
duncanmcclean wants to merge 1 commit into
6.xfrom
prevent-stale-entry-restores
Open

[6.x] Prevent deleted entries from being restored#15310
duncanmcclean wants to merge 1 commit into
6.xfrom
prevent-stale-entry-restores

Conversation

@duncanmcclean

Copy link
Copy Markdown
Member

This pull request fixes an issue where a long-lived worker could recreate a deleted entry as an empty stub file.

This was happening because a stale in-memory Stache path could outlive the shared cached entry. A cache miss then attempted to parse the removed file, which generated and wrote a new entry.

This PR fixes it by returning no item when the path no longer exists.

Fixes #15225

@jasonvarga jasonvarga left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning: Value::getItems() (src/Stache/Indexes/Value.php:11) doesn't filter out the null items BasicStore::getItem() can now return. This index class is the default value index for BasicStore-based stores, used whenever a query filters/sorts on a field whose index isn't cached yet — not a rare path.

Trace: getItemsFromFiles()getItemValue(null)(new ResolveValue)(null, $name). Once $name doesn't start with data->, the loop's getItemPartValue(null, $part) falls through every check (is_array, is_scalar, ContainsQueryableValues, method_exists) to return $item->get($name); with $item === null — a fatal error.

Before this PR, the same stale-path race produced a corrupt-but-non-null stub Entry here, so ResolveValue never crashed — just resolved to wrong/empty data. After this PR, the exact race this PR fixes can now hard-crash index building instead of degrading silently. Query\Builder::getItems() (src/Stache/Query/Builder.php:146) already does ->filter()->values() and is unaffected — Value::getItems() is the one unguarded consumer of getItemsFromFiles().

Suggested fix: $this->store->getItemsFromFiles()->filter()->map(...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleted entries can be resurrected as empty stub files by a long-lived queue worker (Stache/Blink staleness + watcher=false)

2 participants