Repository navigation
fix: validate region ownership of wanted item targets - #527
Open
PluginsKers wants to merge 1 commit into
Open
PluginsKers wants to merge 1 commit into
PluginsKers 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.
GoToWantedItemreads a cached item without checking whether the current region still owns it. For allays,EntityTargetEventcan also replace the target or begin transferring the original item before the behavior callsCraftItem.getHandle. That checked handle access throws for an unowned target;Level.guardEntityTickthen discards the allay.Check ownership of the remembered item before the predicate, distance, and world-border reads. Recheck the event's item target after cancellation and non-item handling, before
getHandle.Bukkit.isOwnedByCurrentRegionusesCraftServer -> CraftEntity.getHandleRaw -> TickThread.isTickThreadFor, so the check does not itself invoke the checked handle accessor. Rejecting an unowned item returns false before creatingEntityTrackerinstances or changing look/walk memories. Existing cancellation, null/non-item memory erasure, local replacement, pickup eligibility, and walk-interruption behavior remain unchanged.The shared callers are
AllayAi.initIdleActivity,PiglinAi.initAdmireItemActivity, andVillagerGoalPackages.getCorePackage. The production stack specifically identifies the post-eventCraftItem.getHandlefailure; it does not distinguish a stale original target from a plugin replacement. This change covers the consumer and event boundary rather than relying on a sensor-time ownership check.Complete captured production call chain (26.2-7-14b7fee, entry to failure)
Original exception stack. These method names and line numbers belong to the captured 26.2 build.
597e41371802648e0455fc1571192ee1c83fca2band this PR's commit for the fixed variant. Use the same probe JAR in fresh, loopback-only server directories and runjava -Xms1G -Xmx2G -Duser.language=en -Dfolia.probe=item -jar folia.jar --nogui.(0, 0)and(512, 0)active in separate regions. On their owner regions, create an allay at(3.5, 65.5, 3.5)holding stone, a local stone item, and a remote stone item. Disable autonomous AI, enable item pickup, erase pickup cooldown, and seed wanted/look/walk memories with known references.EntityTargetEventlistener for that allay andCLOSEST_ENTITY. Replaceevent.targetwith the remote item's Bukkit wrapper. InvokeGoToWantedItem.create(body -> true, 1.75f, true, 32).tryStartinsidelevel.guardEntityTickon the allay's region. The unchanged server throws atCraftItem.getHandleand removes the allay. With the guards, it returns false, keeps the allay, and preserves the seeded memories.teleportAsyncon the original item while leavingevent.targetunchanged. The latter case asserts that the item has relinquished region ownership before the listener returns.The production failure identifies the post-event checked accessor. The fixture establishes each ownership precondition explicitly; it does not claim to reconstruct the production listener or the original source of stale memory.
The current 26.3 failure stack, baseline log, and fixed log are pinned alongside the fixture. The recorded fixed runtime contains the three independent guards; this PR contains only the item-target change. The reproductions invoke real behaviors on owner threads with autonomous AI disabled; separate single-fix runtime JARs and complete autonomous gameplay runs were not recorded. Read assertion markers in the logs, since the fixture shuts down normally even after an assertion fails.
AI tools assisted the investigation, implementation, reproduction, and this description.