Skip to content

Commit

Permalink
Fixes to tab list
Browse files Browse the repository at this point in the history 
Remove entries on ClientboundPlayerInfoRemovePacket, this was
missed after Mojang split the packet to two.

Prevent client from receiving player info packets
related to clients that are not present in the tab list.
Previously we would leak the people who are in vanish.
  • Loading branch information
@aromaa
aromaa committed Jan 6, 2025
1 parent 63d0f56 commit cd61fe8
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 13 deletions.
49 changes: 45 additions & 4 deletions src/main/java/org/spongepowered/common/entity/player/tab/SpongeTabList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import net.minecraft.network.protocol.game.ClientboundPlayerInfoUpdatePacket;
import net.minecraft.network.protocol.game.ClientboundTabListPacket;
import net.minecraft.world.level.GameType;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.spongepowered.api.entity.living.player.gamemode.GameMode;
import org.spongepowered.api.entity.living.player.server.ServerPlayer;
Expand All @@ -43,6 +44,7 @@
import org.spongepowered.common.profile.SpongeGameProfile;
import org.spongepowered.common.util.Preconditions;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
Expand Down Expand Up @@ -205,18 +207,26 @@ void sendUpdate(final TabListEntry entry, final EnumSet<ClientboundPlayerInfoUpd
* <p>This method should not be called manually, it is automatically
* called in the player's network connection when the packet is sent.</p>
*
* <p>We also filter out entries that the client does not see.</p>
*
* @param packet The packet to process
*/
@SuppressWarnings("ConstantConditions")
public void updateEntriesOnSend(final ClientboundPlayerInfoUpdatePacket packet) {
public @Nullable ClientboundPlayerInfoUpdatePacket updateEntriesOnSend(final ClientboundPlayerInfoUpdatePacket packet) {
@MonotonicNonNull List<ClientboundPlayerInfoUpdatePacket.Entry> filteredEntries = null;
final EnumSet<ClientboundPlayerInfoUpdatePacket.Action> actions = packet.actions();
for (final ClientboundPlayerInfoUpdatePacket.Entry update : packet.entries()) {
for (int i = 0; i < packet.entries().size(); i++) {
final ClientboundPlayerInfoUpdatePacket.Entry update = packet.entries().get(i);
if (actions.contains(ClientboundPlayerInfoUpdatePacket.Action.ADD_PLAYER)) {
// If an entry with the same id exists nothing will be done
this.addEntry(update);
}

this.entry(update.profileId()).ifPresent(entry -> {
final @Nullable TabListEntry entry = this.entry(update.profileId()).orElse(null);
if (entry != null) {
if (filteredEntries != null) {
filteredEntries.add(update);
}
if (actions.contains(ClientboundPlayerInfoUpdatePacket.Action.UPDATE_DISPLAY_NAME)) {
((SpongeTabListEntry) entry).updateWithoutSend();
entry.setDisplayName(update.displayName() == null ? null : SpongeAdventure.asAdventure(update.displayName()));
Expand All @@ -233,8 +243,39 @@ public void updateEntriesOnSend(final ClientboundPlayerInfoUpdatePacket packet)
((SpongeTabListEntry) entry).updateWithoutSend();
entry.setListed(update.listed());
}
});
} else {
if (packet.entries().size() == 1) {
return null;
}
filteredEntries = new ArrayList<>(packet.entries().subList(0, i));
}
}
if (filteredEntries == null) {
return packet;
}
final var filteredPacket = new ClientboundPlayerInfoUpdatePacket(packet.actions(), List.of());
((ClientboundPlayerInfoUpdatePacketAccessor)filteredPacket).accessor$entries(filteredEntries);
return filteredPacket;
}

public @Nullable ClientboundPlayerInfoRemovePacket updateEntriesOnSend(final ClientboundPlayerInfoRemovePacket packet) {
@MonotonicNonNull List<UUID> filteredProfileIds = null;
for (int i = 0; i < packet.profileIds().size(); i++) {
final UUID uniqueId = packet.profileIds().get(i);
final TabListEntry entry = this.entries.remove(uniqueId);
if (entry == null) {
if (packet.profileIds().size() == 1) {
return null;
}
filteredProfileIds = new ArrayList<>(packet.profileIds().subList(0, i));
} else if (filteredProfileIds != null) {
filteredProfileIds.add(uniqueId);
}
}
if (filteredProfileIds == null) {
return packet;
}
return new ClientboundPlayerInfoRemovePacket(filteredProfileIds);
}

@Override
Expand Down
21 changes: 16 additions & 5 deletions ...g/spongepowered/common/mixin/core/server/network/ServerCommonPacketListenerImplMixin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.ModifyVariable;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
import org.spongepowered.common.SpongeCommon;
import org.spongepowered.common.adventure.SpongeAdventure;
Expand Down Expand Up @@ -85,18 +86,28 @@ public abstract class ServerCommonPacketListenerImplMixin implements ServerCommo
private Map<UUID, ResourcePackInfo> impl$resourcePackInfos = new ConcurrentHashMap<>();
private Map<UUID, ResourcePackCallback> impl$resourcePackCallbacks = new ConcurrentHashMap<>();

@Inject(
@ModifyVariable(
method = "send(Lnet/minecraft/network/protocol/Packet;Lnet/minecraft/network/PacketSendListener;)V",
at = @At("HEAD")
at = @At("HEAD"),
argsOnly = true
)
private void impl$onClientboundPacketSend(final Packet<?> packet, final PacketSendListener listener, final CallbackInfo ci) {
this.impl$modifyClientBoundPacket(packet);
private @Nullable Packet<?> impl$onClientboundPacketSend(final Packet<?> packet) {
return this.impl$modifyClientBoundPacket(packet);
}

public void impl$modifyClientBoundPacket(final Packet<?> packet) {
@Inject(method = "send(Lnet/minecraft/network/protocol/Packet;Lnet/minecraft/network/PacketSendListener;)V",
at = @At(value = "INVOKE", target = "Lnet/minecraft/network/protocol/Packet;isTerminal()Z"), cancellable = true)
private void impl$onClientboundPacketSendCancelled(final Packet<?> $$0, final PacketSendListener $$1, final CallbackInfo ci) {
if ($$0 == null) {
ci.cancel();
}
}

public @Nullable Packet<?> impl$modifyClientBoundPacket(final Packet<?> packet) {
if (packet instanceof ClientboundResourcePackPushPacket packPacket) {
this.impl$resourcePackInfos.put(packPacket.id(), ((ClientboundResourcePackPacketBridge) (Object) packPacket).bridge$getPackInfo());
}
return packet;
}

@Inject(method = "handleResourcePackResponse", at = @At(
Expand Down
11 changes: 7 additions & 4 deletions ...org/spongepowered/common/mixin/core/server/network/ServerGamePacketListenerImplMixin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import net.minecraft.network.protocol.game.ClientboundBlockUpdatePacket;
import net.minecraft.network.protocol.game.ClientboundCommandSuggestionsPacket;
import net.minecraft.network.protocol.game.ClientboundMoveVehiclePacket;
import net.minecraft.network.protocol.game.ClientboundPlayerInfoRemovePacket;
import net.minecraft.network.protocol.game.ClientboundPlayerInfoUpdatePacket;
import net.minecraft.network.protocol.game.ServerboundChatCommandSignedPacket;
import net.minecraft.network.protocol.game.ServerboundCommandSuggestionPacket;
Expand Down Expand Up @@ -141,11 +142,13 @@ public abstract class ServerGamePacketListenerImplMixin extends ServerCommonPack
private int impl$ignorePackets;

@Override
public void impl$modifyClientBoundPacket(final Packet<?> packet) {
super.impl$modifyClientBoundPacket(packet);
if (packet instanceof ClientboundPlayerInfoUpdatePacket infoPacket) {
((SpongeTabList) ((ServerPlayer) this.player).tabList()).updateEntriesOnSend(infoPacket);
public @Nullable Packet<?> impl$modifyClientBoundPacket(final Packet<?> packet) {
if (packet instanceof final ClientboundPlayerInfoUpdatePacket infoPacket) {
return ((SpongeTabList) ((ServerPlayer) this.player).tabList()).updateEntriesOnSend(infoPacket);
} else if (packet instanceof final ClientboundPlayerInfoRemovePacket removePacket) {
return ((SpongeTabList) ((ServerPlayer) this.player).tabList()).updateEntriesOnSend(removePacket);
}
return super.impl$modifyClientBoundPacket(packet);
}

@Override
Expand Down

0 comments on commit cd61fe8

Please sign in to comment.