Have TranslatableContents throw when fed improper arguments in dev

Closes neoforged#703 Also fixed several spots in NeoForge where we were passing bad arguments as well. Note: the track command's crash from bad argument passed to Translatable will be fixed in this PR: neoforged#915
TelepathicGrunt · May 4, 2024 · efc3c13 · efc3c13
1 parent 8bdd59d
commit efc3c13
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 16 deletions.
diff --git a/patches/net/minecraft/network/chat/contents/TranslatableContents.java.patch b/patches/net/minecraft/network/chat/contents/TranslatableContents.java.patch
@@ -1,14 +1,28 @@
 --- a/net/minecraft/network/chat/contents/TranslatableContents.java
 +++ b/net/minecraft/network/chat/contents/TranslatableContents.java
-@@ -140,6 +_,11 @@
-                 j = l;
-             }
+@@ -81,6 +_,13 @@
+         this.key = p_265775_;
+         this.fallback = p_265204_;
+         this.args = p_265752_;
++        if (!net.neoforged.fml.loading.FMLEnvironment.production) {
++            for (Object arg : this.args) {
++                if (!(arg instanceof Component) && !isAllowedPrimitiveArgument(arg)) {
++                    throw new IllegalArgumentException("TranslatableContents's arguments must be a Component, Number, Boolean, or String. Was given " + arg + " for " + this.key);
++                }
++            }
++        }
+     }
 
+     @Override
+@@ -138,6 +_,11 @@
+                 }
+
+                 j = l;
++            }
++
 +            if (j == 0) {
 +                // Neo has some special formatting handlers defined in I18nExtension, use those if no %s replacements present.
 +                j = net.neoforged.neoforge.internal.TextComponentMessageFormatHandler.handle(this, p_237517_, this.args, p_237516_);
-+            }
-+
+             }
+ 
              if (j < p_237516_.length()) {
-                 String s3 = p_237516_.substring(j);
-                 if (s3.indexOf(37) != -1) {
diff --git a/src/main/java/net/neoforged/neoforge/forge/snapshots/ForgeSnapshotsModClient.java b/src/main/java/net/neoforged/neoforge/forge/snapshots/ForgeSnapshotsModClient.java
@@ -17,7 +17,7 @@ public static void renderMainMenuWarning(String neoForgeVersion, GuiGraphics gra
             graphics.drawCenteredString(font, Component.translatable("loadwarning.neoforge.prbuild"), width / 2, 4 + (font.lineHeight + 1) / 2, 0xFFFFFF | alpha);
         } else if (neoForgeVersion.contains("-beta")) {
             // Render a warning at the top of the screen
-            Component line = Component.translatable("neoforge.update.beta.1", ChatFormatting.RED, ChatFormatting.RESET).withStyle(ChatFormatting.RED);
+            Component line = Component.translatable("neoforge.update.beta.1", ChatFormatting.RED.toString(), ChatFormatting.RESET.toString()).withStyle(ChatFormatting.RED);
             graphics.drawCenteredString(font, line, width / 2, 4 + (0 * (font.lineHeight + 1)), 0xFFFFFF | alpha);
             line = Component.translatable("neoforge.update.beta.2");
             graphics.drawCenteredString(font, line, width / 2, 4 + (1 * (font.lineHeight + 1)), 0xFFFFFF | alpha);

diff --git a/src/main/java/net/neoforged/neoforge/network/handlers/ClientPayloadHandler.java b/src/main/java/net/neoforged/neoforge/network/handlers/ClientPayloadHandler.java
@@ -131,7 +131,7 @@ public static void handle(AuxiliaryLightDataPayload msg, IPayloadContext context
                 manager.handleLightDataSync(msg.entries());
             }
         } catch (Throwable t) {
-            context.disconnect(Component.translatable("neoforge.network.aux_light_data.failed", msg.pos(), t.getMessage()));
+            context.disconnect(Component.translatable("neoforge.network.aux_light_data.failed", msg.pos().toString(), t.getMessage()));
         }
     }
 

diff --git a/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java b/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java
@@ -154,9 +154,9 @@ private static List<NegotiableNetworkComponent> buildDisabledOptionalComponents(
     public static Optional<ComponentNegotiationResult> validateComponent(NegotiableNetworkComponent left, NegotiableNetworkComponent right, String requestingSide) {
         if (left.flow().isPresent()) {
             if (right.flow().isEmpty()) {
-                return Optional.of(new ComponentNegotiationResult(false, Component.translatable("neoforge.network.negotiation.failure.flow.%s.missing".formatted(requestingSide), left.flow().get())));
+                return Optional.of(new ComponentNegotiationResult(false, Component.translatable("neoforge.network.negotiation.failure.flow.%s.missing".formatted(requestingSide), left.flow().get().toString())));
             } else if (left.flow().get() != right.flow().get()) {
-                return Optional.of(new ComponentNegotiationResult(false, Component.translatable("neoforge.network.negotiation.failure.flow.%s.mismatch".formatted(requestingSide), left.flow().get(), right.flow().get())));
+                return Optional.of(new ComponentNegotiationResult(false, Component.translatable("neoforge.network.negotiation.failure.flow.%s.mismatch".formatted(requestingSide), left.flow().get().toString(), right.flow().get().toString())));
             }
         }
 

diff --git a/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java b/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java
@@ -246,14 +246,14 @@ public static void handleModdedPayload(ServerCommonPacketListener listener, Serv
             // Check if the channel should even be processed.
             if (channel == null && !hasAdhocChannel(listener.protocol(), context.payloadId(), PacketFlow.SERVERBOUND)) {
                 LOGGER.warn("Received a modded payload {} with an unknown or unaccepted channel; disconnecting.", context.payloadId());
-                listener.disconnect(Component.translatable("multiplayer.disconnect.incompatible", "NeoForge %s (No Channel for %s)".formatted(NeoForgeVersion.getVersion(), context.payloadId())));
+                listener.disconnect(Component.translatable("multiplayer.disconnect.incompatible", "NeoForge %s (No Channel for %s)".formatted(NeoForgeVersion.getVersion(), context.payloadId().toString())));
                 return;
             }
 
             PayloadRegistration registration = PAYLOAD_REGISTRATIONS.get(listener.protocol()).get(context.payloadId());
             if (registration == null) {
                 LOGGER.error("Received a modded payload {} with no registration; disconnecting.", context.payloadId());
-                listener.disconnect(Component.translatable("multiplayer.disconnect.incompatible", "NeoForge %s (No Handler for %s)".formatted(NeoForgeVersion.getVersion(), context.payloadId())));
+                listener.disconnect(Component.translatable("multiplayer.disconnect.incompatible", "NeoForge %s (No Handler for %s)".formatted(NeoForgeVersion.getVersion(), context.payloadId().toString())));
                 dumpStackToLog(); // This case is only likely when handling packets without serialization, i.e. from a compound listener, so this can help debug why.
                 return;
             }
@@ -292,14 +292,14 @@ public static void handleModdedPayload(ClientCommonPacketListener listener, Clie
             // Check if the channel should even be processed.
             if (channel == null && !hasAdhocChannel(listener.protocol(), packet.payload().type().id(), PacketFlow.CLIENTBOUND)) {
                 LOGGER.warn("Received a modded payload with an unknown or unaccepted channel; disconnecting.");
-                listener.getConnection().disconnect(Component.translatable("multiplayer.disconnect.incompatible", "NeoForge %s (No Channel for %s)".formatted(NeoForgeVersion.getVersion(), context.payloadId())));
+                listener.getConnection().disconnect(Component.translatable("multiplayer.disconnect.incompatible", "NeoForge %s (No Channel for %s)".formatted(NeoForgeVersion.getVersion(), context.payloadId().toString())));
                 return;
             }
 
             PayloadRegistration registration = PAYLOAD_REGISTRATIONS.get(listener.protocol()).get(context.payloadId());
             if (registration == null) {
                 LOGGER.error("Received a modded payload with no registration; disconnecting.");
-                listener.getConnection().disconnect(Component.translatable("multiplayer.disconnect.incompatible", "NeoForge %s (No Handler for %s)".formatted(NeoForgeVersion.getVersion(), context.payloadId())));
+                listener.getConnection().disconnect(Component.translatable("multiplayer.disconnect.incompatible", "NeoForge %s (No Handler for %s)".formatted(NeoForgeVersion.getVersion(), context.payloadId().toString())));
                 dumpStackToLog(); // This case is only likely when handling packets without serialization, i.e. from a compound listener, so this can help debug why.
                 return;
             }

diff --git a/src/main/java/net/neoforged/neoforge/registries/ClientRegistryManager.java b/src/main/java/net/neoforged/neoforge/registries/ClientRegistryManager.java
@@ -41,7 +41,7 @@ public static <R> void handleDataMapSync(final RegistryDataMapSyncPayload<R> pay
                 registry.dataMaps.clear();
                 payload.dataMaps().forEach((attachKey, maps) -> registry.dataMaps.put(RegistryManager.getDataMap(payload.registryKey(), attachKey), Collections.unmodifiableMap(maps)));
             } catch (Throwable t) {
-                context.disconnect(Component.translatable("neoforge.network.data_maps.failed", payload.registryKey().location(), t.getMessage()));
+                context.disconnect(Component.translatable("neoforge.network.data_maps.failed", payload.registryKey().location().toString(), t.getMessage()));
                 LOGGER.error("Failed to handle registry data map sync: ", t);
             }
         });