Fix: Ignore invalid block entity data sent by Java server (GeyserMC#4766

) * Proper block entity checks; ignore invalid block entity data sent by Java server * fix intelliJ warning about potentially null block state * Use auto-generated block entity types instead of hardcoding them * undo some diff * Update BlockRegistryPopulator.java * Access block entity type of state by getting the block first * deprecate JavaBlockState#hasBlockEntity * Simplify check * Add type check in JavaBlockEntityDataTranslator, ensure deprecated setBlockEntity() method still sets piston behavior * nullability annotations * yeet duplicate check
Shanwer · Jun 19, 2024 · 126d56d · 126d56d
1 parent 34158f9
commit 126d56d
Show file tree

Hide file tree

Showing 16 changed files with 226 additions and 225 deletions.
diff --git a/api/src/main/java/org/geysermc/geyser/api/block/custom/nonvanilla/JavaBlockState.java b/api/src/main/java/org/geysermc/geyser/api/block/custom/nonvanilla/JavaBlockState.java
@@ -73,7 +73,10 @@ public interface JavaBlockState {
      * Gets whether the block state has block entity
      * 
      * @return whether the block state has block entity
+     * @deprecated Does not have an effect. If you were using this to
+     * set piston behavior, use {@link #pistonBehavior()} instead.
      */
+    @Deprecated(forRemoval = true)
     boolean hasBlockEntity();
 
     /**
@@ -104,6 +107,11 @@ interface Builder {
 
         Builder pistonBehavior(@Nullable String pistonBehavior);
 
+        /**
+         * @deprecated Does not have an effect. If you were using this to
+         *      * set piston behavior, use {@link #pistonBehavior(String)} instead.
+         */
+        @Deprecated(forRemoval = true)
         Builder hasBlockEntity(boolean hasBlockEntity);
 
         JavaBlockState build();

diff --git a/core/src/main/java/org/geysermc/geyser/level/block/Blocks.java b/core/src/main/java/org/geysermc/geyser/level/block/Blocks.java
diff --git a/core/src/main/java/org/geysermc/geyser/level/block/GeyserJavaBlockState.java b/core/src/main/java/org/geysermc/geyser/level/block/GeyserJavaBlockState.java
@@ -16,7 +16,6 @@ public class GeyserJavaBlockState implements JavaBlockState {
     boolean canBreakWithHand;
     String pickItem;
     String pistonBehavior;
-    boolean hasBlockEntity;
 
     private GeyserJavaBlockState(Builder builder) {
         this.identifier = builder.identifier;
@@ -28,7 +27,6 @@ private GeyserJavaBlockState(Builder builder) {
         this.canBreakWithHand = builder.canBreakWithHand;
         this.pickItem = builder.pickItem;
         this.pistonBehavior = builder.pistonBehavior;
-        this.hasBlockEntity = builder.hasBlockEntity;
     }
 
     @Override
@@ -76,9 +74,10 @@ public boolean canBreakWithHand() {
         return pistonBehavior;
     }
 
+    @SuppressWarnings("removal")
     @Override
     public boolean hasBlockEntity() {
-        return hasBlockEntity;
+        return false;
     }
 
     public static class Builder implements JavaBlockState.Builder {
@@ -91,7 +90,6 @@ public static class Builder implements JavaBlockState.Builder {
         private boolean canBreakWithHand;
         private String pickItem;
         private String pistonBehavior;
-        private boolean hasBlockEntity;
 
         @Override
         public Builder identifier(@NonNull String identifier) {
@@ -147,9 +145,13 @@ public Builder pistonBehavior(@Nullable String pistonBehavior) {
             return this;
         }
 
+        @SuppressWarnings("removal")
         @Override
         public Builder hasBlockEntity(boolean hasBlockEntity) {
-            this.hasBlockEntity = hasBlockEntity;
+            // keep the current behavior
+            if (this.pistonBehavior == null && hasBlockEntity) {
+                this.pistonBehavior = "BLOCK";
+            }
             return this;
         }
 

diff --git a/core/src/main/java/org/geysermc/geyser/level/block/type/Block.java b/core/src/main/java/org/geysermc/geyser/level/block/type/Block.java
@@ -29,6 +29,7 @@
 import it.unimi.dsi.fastutil.ints.IntList;
 import net.kyori.adventure.key.Key;
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.cloudburstmc.math.vector.Vector3i;
 import org.cloudburstmc.protocol.bedrock.data.definitions.BlockDefinition;
 import org.cloudburstmc.protocol.bedrock.packet.UpdateBlockPacket;
@@ -41,6 +42,7 @@
 import org.geysermc.geyser.registry.BlockRegistries;
 import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.mcprotocollib.protocol.data.game.item.ItemStack;
+import org.geysermc.mcprotocollib.protocol.data.game.level.block.BlockEntityType;
 import org.intellij.lang.annotations.Subst;
 
 import java.util.*;
@@ -55,7 +57,7 @@ public class Block {
      * Can you harvest this with your hand.
      */
     private final boolean requiresCorrectToolForDrops;
-    private final boolean hasBlockEntity;
+    private final @Nullable BlockEntityType blockEntityType;
     private final float destroyTime;
     private final @NonNull PistonBehavior pushReaction;
     /**
@@ -75,7 +77,7 @@ public class Block {
     public Block(@Subst("empty") String javaIdentifier, Builder builder) {
         this.javaIdentifier = Key.key(javaIdentifier);
         this.requiresCorrectToolForDrops = builder.requiresCorrectToolForDrops;
-        this.hasBlockEntity = builder.hasBlockEntity;
+        this.blockEntityType = builder.blockEntityType;
         this.destroyTime = builder.destroyTime;
         this.pushReaction = builder.pushReaction;
         this.pickItem = builder.pickItem;
@@ -181,7 +183,12 @@ public boolean requiresCorrectToolForDrops() {
     }
 
     public boolean hasBlockEntity() {
-        return hasBlockEntity;
+        return blockEntityType != null;
+    }
+
+    @Nullable
+    public BlockEntityType blockEntityType() {
+        return blockEntityType;
     }
 
     public float destroyTime() {
@@ -227,7 +234,7 @@ public static Builder builder() {
     public static final class Builder {
         private final Map<Property<?>, List<Comparable<?>>> states = new LinkedHashMap<>();
         private boolean requiresCorrectToolForDrops = false;
-        private boolean hasBlockEntity = false;
+        private BlockEntityType blockEntityType = null;
         private PistonBehavior pushReaction = PistonBehavior.NORMAL;
         private float destroyTime;
         private Supplier<Item> pickItem;
@@ -271,8 +278,8 @@ public Builder requiresCorrectToolForDrops() {
             return this;
         }
 
-        public Builder setBlockEntity() {
-            this.hasBlockEntity = true;
+        public Builder setBlockEntity(BlockEntityType blockEntityType) {
+            this.blockEntityType = blockEntityType;
             return this;
         }
 

diff --git a/core/src/main/java/org/geysermc/geyser/level/block/type/BlockState.java b/core/src/main/java/org/geysermc/geyser/level/block/type/BlockState.java
@@ -65,14 +65,6 @@ public <T extends Comparable<T>> T getValueNullable(Property<T> property) {
         return (T) get(property);
     }
 
-    public boolean getValue(Property<Boolean> property, boolean def) {
-        var value = get(property);
-        if (value == null) {
-            return def;
-        }
-        return (Boolean) value;
-    }
-
     public <T extends Comparable<T>> T getValue(Property<T> property, T def) {
         var value = get(property);
         if (value == null) {

diff --git a/core/src/main/java/org/geysermc/geyser/registry/populator/BlockRegistryPopulator.java b/core/src/main/java/org/geysermc/geyser/registry/populator/BlockRegistryPopulator.java
@@ -435,9 +435,6 @@ private static void registerJavaBlocks() {
                 if (!javaBlockState.canBreakWithHand()) {
                     builder.requiresCorrectToolForDrops();
                 }
-                if (javaBlockState.hasBlockEntity()) {
-                    builder.setBlockEntity();
-                }
                 String cleanJavaIdentifier = BlockUtils.getCleanIdentifier(javaBlockState.identifier());
                 String pickItem = javaBlockState.pickItem();
                 Block block = new Block(cleanJavaIdentifier, builder) {

diff --git a/...ava/org/geysermc/geyser/translator/level/block/entity/BrushableBlockEntityTranslator.java b/...ava/org/geysermc/geyser/translator/level/block/entity/BrushableBlockEntityTranslator.java
@@ -37,7 +37,6 @@
 
 @BlockEntity(type = BlockEntityType.BRUSHABLE_BLOCK)
 public class BrushableBlockEntityTranslator extends BlockEntityTranslator implements RequiresBlockState {
-
     @Override
     public void translateTag(GeyserSession session, NbtMapBuilder bedrockNbt, @Nullable NbtMap javaNbt, BlockState blockState) {
         if (javaNbt == null) {

diff --git a/.../org/geysermc/geyser/translator/level/block/entity/DecoratedPotBlockEntityTranslator.java b/.../org/geysermc/geyser/translator/level/block/entity/DecoratedPotBlockEntityTranslator.java
@@ -34,7 +34,6 @@
 
 @BlockEntity(type = BlockEntityType.DECORATED_POT)
 public class DecoratedPotBlockEntityTranslator extends BlockEntityTranslator {
-
     @Override
     public void translateTag(GeyserSession session, NbtMapBuilder bedrockNbt, NbtMap javaNbt, BlockState blockState) {
         if (javaNbt == null) {

diff --git a/...a/org/geysermc/geyser/translator/level/block/entity/DoubleChestBlockEntityTranslator.java b/...a/org/geysermc/geyser/translator/level/block/entity/DoubleChestBlockEntityTranslator.java
@@ -27,7 +27,6 @@
 
 import org.cloudburstmc.nbt.NbtMap;
 import org.cloudburstmc.nbt.NbtMapBuilder;
-import org.geysermc.geyser.level.block.Blocks;
 import org.geysermc.geyser.level.block.property.ChestType;
 import org.geysermc.geyser.level.block.property.Properties;
 import org.geysermc.geyser.level.block.type.BlockState;
@@ -40,12 +39,8 @@
  */
 @BlockEntity(type = { BlockEntityType.CHEST, BlockEntityType.TRAPPED_CHEST })
 public class DoubleChestBlockEntityTranslator extends BlockEntityTranslator implements RequiresBlockState {
-
     @Override
     public void translateTag(GeyserSession session, NbtMapBuilder bedrockNbt, NbtMap javaNbt, BlockState blockState) {
-        if (!(blockState.is(Blocks.CHEST) || blockState.is(Blocks.TRAPPED_CHEST))) {
-            return;
-        }
         if (blockState.getValue(Properties.CHEST_TYPE) != ChestType.SINGLE) {
             int x = (int) bedrockNbt.get("x");
             int z = (int) bedrockNbt.get("z");

diff --git a/...va/org/geysermc/geyser/translator/level/block/entity/ShulkerBoxBlockEntityTranslator.java b/...va/org/geysermc/geyser/translator/level/block/entity/ShulkerBoxBlockEntityTranslator.java
@@ -30,7 +30,6 @@
 import org.cloudburstmc.nbt.NbtMapBuilder;
 import org.geysermc.geyser.level.block.property.Properties;
 import org.geysermc.geyser.level.block.type.BlockState;
-import org.geysermc.geyser.level.physics.Direction;
 import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.inventory.ShulkerInventoryTranslator;
 import org.geysermc.mcprotocollib.protocol.data.game.level.block.BlockEntityType;
@@ -43,6 +42,6 @@ public class ShulkerBoxBlockEntityTranslator extends BlockEntityTranslator imple
      */
     @Override
     public void translateTag(GeyserSession session, NbtMapBuilder bedrockNbt, @Nullable NbtMap javaNbt, BlockState blockState) {
-        bedrockNbt.putByte("facing", (byte) blockState.getValue(Properties.FACING, Direction.UP).ordinal());
+        bedrockNbt.putByte("facing", (byte) blockState.getValue(Properties.FACING).ordinal());
     }
 }
diff --git a/...in/java/org/geysermc/geyser/translator/level/block/entity/SkullBlockEntityTranslator.java b/...in/java/org/geysermc/geyser/translator/level/block/entity/SkullBlockEntityTranslator.java
@@ -50,7 +50,6 @@
 
 @BlockEntity(type = BlockEntityType.SKULL)
 public class SkullBlockEntityTranslator extends BlockEntityTranslator implements RequiresBlockState {
-
     @Override
     public void translateTag(GeyserSession session, NbtMapBuilder bedrockNbt, NbtMap javaNbt, BlockState blockState) {
         Integer rotation = blockState.getValue(Properties.ROTATION_16);

diff --git a/.../java/org/geysermc/geyser/translator/level/block/entity/SpawnerBlockEntityTranslator.java b/.../java/org/geysermc/geyser/translator/level/block/entity/SpawnerBlockEntityTranslator.java
@@ -39,7 +39,6 @@
 
 @BlockEntity(type = BlockEntityType.MOB_SPAWNER)
 public class SpawnerBlockEntityTranslator extends BlockEntityTranslator {
-
     @Override
     public NbtMap getBlockEntityTag(GeyserSession session, BlockEntityType type, int x, int y, int z, @Nullable NbtMap javaNbt, BlockState blockState) {
         if (javaNbt == null) {

diff --git a/...rg/geysermc/geyser/translator/level/block/entity/StructureBlockBlockEntityTranslator.java b/...rg/geysermc/geyser/translator/level/block/entity/StructureBlockBlockEntityTranslator.java
@@ -39,7 +39,6 @@
 
 @BlockEntity(type = BlockEntityType.STRUCTURE_BLOCK)
 public class StructureBlockBlockEntityTranslator extends BlockEntityTranslator {
-
     @Override
     public NbtMap getBlockEntityTag(GeyserSession session, BlockEntityType type, int x, int y, int z, @Nullable NbtMap javaNbt, BlockState blockState) {
         if (javaNbt == null) {

diff --git a/.../org/geysermc/geyser/translator/level/block/entity/TrialSpawnerBlockEntityTranslator.java b/.../org/geysermc/geyser/translator/level/block/entity/TrialSpawnerBlockEntityTranslator.java
@@ -33,7 +33,6 @@
 
 @BlockEntity(type = BlockEntityType.TRIAL_SPAWNER)
 public class TrialSpawnerBlockEntityTranslator extends BlockEntityTranslator {
-
     @Override
     public void translateTag(GeyserSession session, NbtMapBuilder bedrockNbt, NbtMap javaNbt, BlockState blockState) {
         if (javaNbt == null) {

diff --git a/...ava/org/geysermc/geyser/translator/protocol/java/level/JavaBlockEntityDataTranslator.java b/...ava/org/geysermc/geyser/translator/protocol/java/level/JavaBlockEntityDataTranslator.java
@@ -33,11 +33,11 @@
 import org.cloudburstmc.protocol.bedrock.data.structure.StructureRotation;
 import org.cloudburstmc.protocol.bedrock.packet.ContainerOpenPacket;
 import org.cloudburstmc.protocol.bedrock.packet.UpdateBlockPacket;
+import org.geysermc.geyser.level.block.Blocks;
 import org.geysermc.geyser.level.block.type.BlockState;
 import org.geysermc.geyser.registry.BlockRegistries;
 import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.level.block.entity.BlockEntityTranslator;
-import org.geysermc.geyser.translator.level.block.entity.RequiresBlockState;
 import org.geysermc.geyser.translator.level.block.entity.SkullBlockEntityTranslator;
 import org.geysermc.geyser.translator.protocol.PacketTranslator;
 import org.geysermc.geyser.translator.protocol.Translator;
@@ -59,11 +59,11 @@ public void translate(GeyserSession session, ClientboundBlockEntityDataPacket pa
         BlockEntityTranslator translator = BlockEntityUtils.getBlockEntityTranslator(type);
         // The Java block state is used in BlockEntityTranslator.translateTag() to make up for some inconsistencies
         // between Java block states and Bedrock block entity data
-        BlockState blockState;
-        if (translator instanceof RequiresBlockState) {
-            blockState = BlockRegistries.BLOCK_STATES.get(session.getGeyser().getWorldManager().getBlockAt(session, packet.getPosition()));
-        } else {
-            blockState = BlockRegistries.BLOCK_STATES.get(0); //TODO
+        BlockState blockState = BlockRegistries.BLOCK_STATES.getOrDefault(session.getGeyser().getWorldManager().getBlockAt(session, packet.getPosition()),
+                Blocks.AIR.defaultBlockState());
+
+        if (blockState.block().blockEntityType() != type) {
+            return;
         }
 
         Vector3i position = packet.getPosition();

diff --git a/...org/geysermc/geyser/translator/protocol/java/level/JavaLevelChunkWithLightTranslator.java b/...org/geysermc/geyser/translator/protocol/java/level/JavaLevelChunkWithLightTranslator.java
@@ -39,6 +39,7 @@
 import org.cloudburstmc.protocol.bedrock.packet.LevelChunkPacket;
 import org.geysermc.geyser.entity.type.ItemFrameEntity;
 import org.geysermc.geyser.level.BedrockDimension;
+import org.geysermc.geyser.level.block.Blocks;
 import org.geysermc.geyser.level.block.type.Block;
 import org.geysermc.geyser.level.block.type.BlockState;
 import org.geysermc.geyser.level.chunk.BlockStorage;
@@ -398,29 +399,34 @@ public void translate(GeyserSession session, ClientboundLevelChunkWithLightPacke
 
                 // Get the Java block state ID from block entity position
                 DataPalette section = javaChunks[(y >> 4) - yOffset];
-                BlockState blockState = BlockRegistries.BLOCK_STATES.get(section.get(x, y & 0xF, z));
+                BlockState blockState = BlockRegistries.BLOCK_STATES.getOrDefault(section.get(x, y & 0xF, z), Blocks.AIR.defaultBlockState());
 
                 // Note that, since 1.20.5, tags can be null, but Bedrock still needs a default tag to render the item
                 // Also, some properties - like banner base colors - are part of the tag and is processed here.
                 BlockEntityTranslator blockEntityTranslator = BlockEntityUtils.getBlockEntityTranslator(type);
-                bedrockBlockEntities.add(blockEntityTranslator.getBlockEntityTag(session, type, x + chunkBlockX, y, z + chunkBlockZ, tag, blockState));
-
-                // Check for custom skulls
-                if (session.getPreferencesCache().showCustomSkulls() && type == BlockEntityType.SKULL && tag != null && tag.containsKey("profile")) {
-                    BlockDefinition blockDefinition = SkullBlockEntityTranslator.translateSkull(session, tag, Vector3i.from(x + chunkBlockX, y, z + chunkBlockZ), blockState);
-                    if (blockDefinition != null) {
-                        int bedrockSectionY = (y >> 4) - (bedrockDimension.minY() >> 4);
-                        int subChunkIndex = (y >> 4) + (bedrockDimension.minY() >> 4);
-                        if (0 <= bedrockSectionY && bedrockSectionY < maxBedrockSectionY) {
-                            // Custom skull is in a section accepted by Bedrock
-                            GeyserChunkSection bedrockSection = sections[bedrockSectionY];
-                            IntList palette = bedrockSection.getBlockStorageArray()[0].getPalette();
-                            if (palette instanceof IntImmutableList || palette instanceof IntLists.Singleton) {
-                                // TODO there has to be a better way to expand the palette .-.
-                                bedrockSection = bedrockSection.copy(subChunkIndex);
-                                sections[bedrockSectionY] = bedrockSection;
+
+                // The Java server can send block entity data for blocks that aren't actually those blocks.
+                // A Java client ignores these
+                if (type == blockState.block().blockEntityType()) {
+                    bedrockBlockEntities.add(blockEntityTranslator.getBlockEntityTag(session, type, x + chunkBlockX, y, z + chunkBlockZ, tag, blockState));
+
+                    // Check for custom skulls
+                    if (session.getPreferencesCache().showCustomSkulls() && type == BlockEntityType.SKULL && tag != null && tag.containsKey("profile")) {
+                        BlockDefinition blockDefinition = SkullBlockEntityTranslator.translateSkull(session, tag, Vector3i.from(x + chunkBlockX, y, z + chunkBlockZ), blockState);
+                        if (blockDefinition != null) {
+                            int bedrockSectionY = (y >> 4) - (bedrockDimension.minY() >> 4);
+                            int subChunkIndex = (y >> 4) + (bedrockDimension.minY() >> 4);
+                            if (0 <= bedrockSectionY && bedrockSectionY < maxBedrockSectionY) {
+                                // Custom skull is in a section accepted by Bedrock
+                                GeyserChunkSection bedrockSection = sections[bedrockSectionY];
+                                IntList palette = bedrockSection.getBlockStorageArray()[0].getPalette();
+                                if (palette instanceof IntImmutableList || palette instanceof IntLists.Singleton) {
+                                    // TODO there has to be a better way to expand the palette .-.
+                                    bedrockSection = bedrockSection.copy(subChunkIndex);
+                                    sections[bedrockSectionY] = bedrockSection;
+                                }
+                                bedrockSection.setFullBlock(x, y & 0xF, z, 0, blockDefinition.getRuntimeId());
                             }
-                            bedrockSection.setFullBlock(x, y & 0xF, z, 0, blockDefinition.getRuntimeId());
                         }
                     }
                 }