fix: lift rejection only re-sends items the requester was sent

Mobile.Lift resolves the item from a client-supplied serial and, on
rejection, re-sent it to the requester unconditionally.

Item.IsSentTo(Mobile) answers whether a client is expected to hold an
item, using the same recipient rule as the adds in ProcessDelta: for a
private container child, the root mobile, the secure-trade parties, or
the container's Openers (trade parties and openers re-checked for map
and range), each gated on CanSee; otherwise CanSee and in update range.
The private-child guard is shared with SendRemovePacket through
TryGetPrivateParent. Lift's rejection path always sends the lift-reject
packet, but resyncs the item only when it is undeleted and IsSentTo the
requester.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: kamronbatman

Projects/Server.Tests/Tests/Items/LiftRejectResyncTests.cs

@@ -0,0 +1,250 @@
+using System;
+using System.Buffers;
+using Server.Accounting;
+using Server.Items;
+using Server.Network;
+using Server.Tests.Network;
+using Xunit;
+
+namespace Server.Tests;
+
+[Collection("Sequential Server Tests")]
+public class LiftRejectResyncTests
+{
+    private static readonly Point3D _baseLoc = new(2200, 2200, 0);
+
+    private static (NetState, Mobile) CreateClient(Point3D location)
+    {
+        var ns = PacketTestUtilities.CreateTestNetState();
+        ns.Account = new MockAccount();
+
+        var mobile = new Mobile(World.NewMobile);
+        mobile.DefaultMobileInit();
+        ns.Mobile = mobile;
+        mobile.NetState = ns;
+        mobile.MoveToWorld(location, Map.Felucca);
+        return (ns, mobile);
+    }
+
+    private static void DisposeClient(NetState ns, Mobile mobile)
+    {
+        ns.Mobile = null;
+        ns.Dispose();
+        mobile.Delete();
+    }
+
+    private static bool ReceivedContentUpdate(NetState ns, Serial serial)
+    {
+        Span<byte> expected = stackalloc byte[5];
+        var writer = new SpanWriter(expected);
+        writer.Write((byte)0x25); // ContainerContentUpdate packet ID
+        writer.Write(serial);
+        return ns.SendBuffer.GetReadSpan().IndexOf(expected) >= 0;
+    }
+
+    private static bool ReceivedWorldItem(NetState ns, Item item)
+    {
+        Span<byte> expected = stackalloc byte[OutgoingEntityPackets.MaxWorldEntityPacketLength];
+        var length = OutgoingItemPackets.CreateWorldItem(expected, item);
+        return ns.SendBuffer.GetReadSpan().IndexOf(expected[..length]) >= 0;
+    }
+
+    [Fact]
+    public void PrivateNestedItem_RejectedLift_DoesNotResyncBystander()
+    {
+        var (ownerNs, owner) = CreateClient(_baseLoc);
+        var (bystanderNs, bystander) = CreateClient(new Point3D(_baseLoc.X + 1, _baseLoc.Y, 0));
+
+        var backpack = new Container(0xE75) { Layer = Layer.Backpack };
+        owner.AddItem(backpack);
+        var pouch = new Container(0xE75);
+        backpack.DropItem(pouch);
+        var item = new Item(0x1234);
+        pouch.DropItem(item);
+
+        try
+        {
+            bystander.Lift(item, item.Amount, out var rejected, out _);
+
+            // Whichever check trips first (CheckNonlocalLift, accessibility, ...), the bystander was
+            // never sent this private child, so the rejection must not resynchronize it to them.
+            Assert.True(rejected);
+            Assert.False(ReceivedContentUpdate(bystanderNs, item.Serial));
+        }
+        finally
+        {
+            DisposeClient(ownerNs, owner);
+            DisposeClient(bystanderNs, bystander);
+        }
+    }
+
+    [Fact]
+    public void OwnerAlreadyHolding_RejectedLift_ResyncsPrivateChildToOwner()
+    {
+        var (ownerNs, owner) = CreateClient(_baseLoc);
+
+        var backpack = new Container(0xE75) { Layer = Layer.Backpack };
+        owner.AddItem(backpack);
+        var item = new Item(0x1234);
+        backpack.DropItem(item);
+
+        owner.Holding = new Item(0x1);
+
+        try
+        {
+            owner.Lift(item, item.Amount, out var rejected, out var reject);
+
+            Assert.True(rejected);
+            Assert.Equal(LRReason.AreHolding, reject);
+            Assert.True(ReceivedContentUpdate(ownerNs, item.Serial));
+        }
+        finally
+        {
+            owner.Holding?.Delete();
+            DisposeClient(ownerNs, owner);
+        }
+    }
+
+    [Fact]
+    public void GroundItemOutOfRange_RejectedLift_DoesNotResyncRequester()
+    {
+        var (requesterNs, requester) = CreateClient(_baseLoc);
+
+        var item = new Item(0x1234);
+        item.MoveToWorld(new Point3D(_baseLoc.X, _baseLoc.Y + 100, 0), Map.Felucca);
+
+        try
+        {
+            requester.Lift(item, item.Amount, out var rejected, out var reject);
+
+            Assert.True(rejected);
+            Assert.Equal(LRReason.OutOfRange, reject);
+            Assert.False(ReceivedWorldItem(requesterNs, item));
+        }
+        finally
+        {
+            DisposeClient(requesterNs, requester);
+            item.Delete();
+        }
+    }
+
+    [Fact]
+    public void GroundContainerOpener_ForcedRejection_ResyncsPrivateChildToOpener()
+    {
+        var (requesterNs, requester) = CreateClient(_baseLoc);
+
+        var container = new Container(0xE75);
+        container.MoveToWorld(new Point3D(_baseLoc.X + 1, _baseLoc.Y, 0), Map.Felucca);
+        var item = new Item(0x1234);
+        container.DropItem(item);
+        container.Openers = [requester];
+
+        requester.Holding = new Item(0x1);
+
+        try
+        {
+            requester.Lift(item, item.Amount, out var rejected, out var reject);
+
+            Assert.True(rejected);
+            Assert.Equal(LRReason.AreHolding, reject);
+            Assert.True(ReceivedContentUpdate(requesterNs, item.Serial));
+        }
+        finally
+        {
+            requester.Holding?.Delete();
+            DisposeClient(requesterNs, requester);
+            container.Delete();
+        }
+    }
+
+    [Fact]
+    public void TradePartner_RejectedLift_ResyncsNestedItemToTradePartner()
+    {
+        var (aNs, a) = CreateClient(_baseLoc);
+        var (bNs, b) = CreateClient(new Point3D(_baseLoc.X + 1, _baseLoc.Y, 0));
+
+        var trade = new SecureTrade(a, b);
+        var pouch = new Container(0xE75);
+        trade.From.Container.DropItem(pouch);
+        var item = new Item(0x1234);
+        pouch.DropItem(item);
+
+        b.Holding = new Item(0x1);
+
+        try
+        {
+            // b is a's trade partner: not the item's root and never an Opener of the pouch, but a
+            // sanctioned recipient of this private child through the trade relationship.
+            b.Lift(item, item.Amount, out var rejected, out var reject);
+
+            Assert.True(rejected);
+            Assert.Equal(LRReason.AreHolding, reject);
+            Assert.True(ReceivedContentUpdate(bNs, item.Serial));
+        }
+        finally
+        {
+            b.Holding?.Delete();
+            trade.From.Container.Delete();
+            trade.To.Container.Delete();
+            DisposeClient(aNs, a);
+            DisposeClient(bNs, b);
+        }
+    }
+
+    [Fact]
+    public void InvisibleItem_RejectedLift_DoesNotResyncToNonStaffOwner()
+    {
+        var (ownerNs, owner) = CreateClient(_baseLoc);
+
+        var backpack = new Container(0xE75) { Layer = Layer.Backpack };
+        owner.AddItem(backpack);
+        var item = new Item(0x1234) { Visible = false };
+        backpack.DropItem(item);
+
+        owner.Holding = new Item(0x1);
+
+        try
+        {
+            owner.Lift(item, item.Amount, out var rejected, out var reject);
+
+            // ProcessDelta never sends an invisible item to a non-staff owner (CanSee gates every
+            // private recipient); the rejection resync must honor the same gate.
+            Assert.True(rejected);
+            Assert.Equal(LRReason.AreHolding, reject);
+            Assert.False(ReceivedContentUpdate(ownerNs, item.Serial));
+        }
+        finally
+        {
+            owner.Holding?.Delete();
+            DisposeClient(ownerNs, owner);
+        }
+    }
+
+    [Fact]
+    public void InvisibleItem_RejectedLift_ResyncsToStaffOwner()
+    {
+        var (ownerNs, owner) = CreateClient(_baseLoc);
+        owner.AccessLevel = AccessLevel.GameMaster;
+
+        var backpack = new Container(0xE75) { Layer = Layer.Backpack };
+        owner.AddItem(backpack);
+        var item = new Item(0x1234) { Visible = false };
+        backpack.DropItem(item);
+
+        owner.Holding = new Item(0x1);
+
+        try
+        {
+            owner.Lift(item, item.Amount, out var rejected, out var reject);
+
+            Assert.True(rejected);
+            Assert.Equal(LRReason.AreHolding, reject);
+            Assert.True(ReceivedContentUpdate(ownerNs, item.Serial));
+        }
+        finally
+        {
+            owner.Holding?.Delete();
+            DisposeClient(ownerNs, owner);
+        }
+    }
+}

Projects/Server/Items/Item.cs

@@ -4051,19 +4051,14 @@ public void SendRemovePacket(Point3D worldLoc)
 
         var removeEntity = stackalloc byte[OutgoingEntityPackets.RemoveEntityLength].InitializePacket();
 
-        if (m_Parent is Container cont && !cont.IsPublicContainer && !cont.IsChildPublic(this))
+        if (TryGetPrivateParent(out var cont))
         {
             // Private children are only ever sent to these recipients (see ProcessDelta); nobody else knows them.
             OutgoingEntityPackets.CreateRemoveEntity(removeEntity, Serial);
 
-            var root = cont.RootParent as Mobile;
-            SendRemoveTo(root, worldLoc, removeEntity);
+            GetPrivateChildRecipients(cont, out var root, out var tradeFrom, out var tradeTo);
 
-            var trade = GetSecureTradeCont()?.Trade;
-            // Trade.From/To are unassigned while SecureTrade's own constructor is still adding the
-            // VirtualCheck to each side's container, so both must stay null-conditional.
-            var tradeFrom = trade?.From?.Mobile;
-            var tradeTo = trade?.To?.Mobile;
+            SendRemoveTo(root, worldLoc, removeEntity);
 
             if (tradeFrom != root)
             {
@@ -4113,6 +4108,58 @@ private void SendRemoveTo(Mobile m, Point3D worldLoc, ReadOnlySpan<byte> removeE
         }
     }
 
+    // Shared with IsSentTo so the private-child guard and recipient set (root/trade/openers) each live
+    // in one place.
+    private bool TryGetPrivateParent(out Container cont)
+    {
+        cont = m_Parent as Container;
+        return cont != null && !cont.IsPublicContainer && !cont.IsChildPublic(this);
+    }
+
+    private void GetPrivateChildRecipients(Container cont, out Mobile root, out Mobile tradeFrom, out Mobile tradeTo)
+    {
+        root = cont.RootParent as Mobile;
+
+        var trade = GetSecureTradeCont()?.Trade;
+        // Trade.From/To are unassigned while SecureTrade's own constructor is still adding the
+        // VirtualCheck to each side's container, so both must stay null-conditional.
+        tradeFrom = trade?.From?.Mobile;
+        tradeTo = trade?.To?.Mobile;
+    }
+
+    /// <summary>
+    /// Whether <paramref name="m"/>'s client is expected to hold this item; gates resends keyed by a
+    /// client-supplied serial.
+    /// </summary>
+    public bool IsSentTo(Mobile m)
+    {
+        if (m == null || Deleted)
+        {
+            return false;
+        }
+
+        if (TryGetPrivateParent(out var cont))
+        {
+            GetPrivateChildRecipients(cont, out var root, out var tradeFrom, out var tradeTo);
+
+            if (m == root)
+            {
+                // Mirrors ProcessDelta's own root gate: CanSee and in range.
+                return m.CanSee(this) && m.InRange(GetWorldLocation(), GetUpdateRange(m));
+            }
+
+            var isTradeOrOpener = m == tradeFrom || m == tradeTo || cont.Openers?.Contains(m) == true;
+
+            // Trade parties and openers are re-validated against ProcessDelta's own gate (CanSee,
+            // current map, update range), so an invisible item or a stale Openers entry that hasn't
+            // been pruned yet never resyncs to them.
+            return isTradeOrOpener && m.CanSee(this) && m.Map == m_Map &&
+                   m.InRange(GetWorldLocation(), GetUpdateRange(m));
+        }
+
+        return m.CanSee(this) && m.InRange(GetWorldLocation(), GetUpdateRange(m));
+    }
+
     public virtual int GetDropSound() => -1;
 
     public Point3D GetWorldLocation()

Projects/Server/Mobiles/Mobile.cs

@@ -5203,7 +5203,8 @@ public virtual void Lift(Item item, int amount, out bool rejected, out LRReason
         {
             state.SendLiftReject(reject);
 
-            if (item.Deleted)
+            // A rejection must not resynchronize state this client was never sent (client-supplied serial).
+            if (item.Deleted || !item.IsSentTo(this))
             {
                 return;
             }