From 5669e60267857da8ee2bbec0089ce8e717b1c4cf Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Fri, 4 Apr 2025 21:12:11 -0400 Subject: [PATCH] Fix double-eval in EffBroadcast by adding an unformatted hook to VString.getMessageComponents --- .../ch/njol/skript/effects/EffBroadcast.java | 43 +++++++++++-------- .../ch/njol/skript/lang/VariableString.java | 30 +++++++++---- ...7718-broadcast evaluates vstrings twice.sk | 8 ++++ 3 files changed, 55 insertions(+), 26 deletions(-) create mode 100644 src/test/skript/tests/regressions/7718-broadcast evaluates vstrings twice.sk diff --git a/src/main/java/ch/njol/skript/effects/EffBroadcast.java b/src/main/java/ch/njol/skript/effects/EffBroadcast.java index 7b41e1f41fd..573c5532545 100644 --- a/src/main/java/ch/njol/skript/effects/EffBroadcast.java +++ b/src/main/java/ch/njol/skript/effects/EffBroadcast.java @@ -1,12 +1,5 @@ package ch.njol.skript.effects; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.regex.Pattern; - import ch.njol.skript.Skript; import ch.njol.skript.doc.Description; import ch.njol.skript.doc.Examples; @@ -24,6 +17,7 @@ import ch.njol.skript.util.Utils; import ch.njol.skript.util.chat.BungeeConverter; import ch.njol.skript.util.chat.ChatMessages; +import ch.njol.skript.util.chat.MessageComponent; import ch.njol.util.Kleenean; import ch.njol.util.StringUtils; import ch.njol.util.coll.CollectionUtils; @@ -35,6 +29,9 @@ import org.bukkit.event.server.BroadcastMessageEvent; import org.jetbrains.annotations.Nullable; +import java.util.*; +import java.util.regex.Pattern; + @Name("Broadcast") @Description("Broadcasts a message to the server.") @Examples({ @@ -83,12 +80,15 @@ public void execute(Event event) { } for (Expression message : getMessages()) { - if (message instanceof VariableString) { - if (!dispatchEvent(getRawString(event, (VariableString) message), receivers)) + if (message instanceof VariableString variableString) { + // get both unformatted and components with single evaluation: https://github.com/SkriptLang/Skript/issues/7718 + StringBuilder unformattedString = new StringBuilder(); + List messageComponents = variableString.getMessageComponents(event, unformattedString); + if (!dispatchEvent(unformattedString.toString(), receivers)) continue; - BaseComponent[] components = BungeeConverter.convert(((VariableString) message).getMessageComponents(event)); + BaseComponent[] components = BungeeConverter.convert(messageComponents); receivers.forEach(receiver -> receiver.spigot().sendMessage(components)); - } else if (message instanceof ExprColoured && ((ExprColoured) message).isUnsafeFormat()) { // Manually marked as trusted + } else if (message instanceof ExprColoured coloured && coloured.isUnsafeFormat()) { // Manually marked as trusted for (Object realMessage : message.getArray(event)) { if (!dispatchEvent(Utils.replaceChatStyles((String) realMessage), receivers)) continue; @@ -97,7 +97,7 @@ public void execute(Event event) { } } else { for (Object messageObject : message.getArray(event)) { - String realMessage = messageObject instanceof String ? (String) messageObject : Classes.toString(messageObject); + String realMessage = messageObject instanceof String string ? string : Classes.toString(messageObject); if (!dispatchEvent(Utils.replaceChatStyles(realMessage), receivers)) continue; receivers.forEach(receiver -> receiver.sendMessage(realMessage)); @@ -123,9 +123,9 @@ private Expression[] getMessages() { * @param message the message * @return true if the dispatched event does not get cancelled */ - @SuppressWarnings("deprecation") + @SuppressWarnings("removal") private static boolean dispatchEvent(String message, List receivers) { - Set recipients = Collections.unmodifiableSet(new HashSet<>(receivers)); + Set recipients = Set.copyOf(receivers); BroadcastMessageEvent broadcastEvent; if (Skript.isRunningMinecraft(1, 14)) { broadcastEvent = new BroadcastMessageEvent(!Bukkit.isPrimaryThread(), message, recipients); @@ -136,11 +136,18 @@ private static boolean dispatchEvent(String message, List receive return !broadcastEvent.isCancelled(); } - @Nullable - private static String getRawString(Event event, Expression string) { - if (string instanceof VariableString) - return ((VariableString) string).toUnformattedString(event); + /** + * Gets the raw string from the expression, replacing colour codes. + * @param event the event + * @param string the expression + * @return the raw string + */ + private static @Nullable String getRawString(Event event, Expression string) { + if (string instanceof VariableString variableString) + return variableString.toUnformattedString(event); String rawString = string.getSingle(event); + if (rawString == null) + return null; rawString = SkriptColor.replaceColorChar(rawString); if (rawString.toLowerCase().contains("&x")) { rawString = StringUtils.replaceAll(rawString, HEX_PATTERN, matchResult -> diff --git a/src/main/java/ch/njol/skript/lang/VariableString.java b/src/main/java/ch/njol/skript/lang/VariableString.java index db6b1173e82..9baf838c481 100644 --- a/src/main/java/ch/njol/skript/lang/VariableString.java +++ b/src/main/java/ch/njol/skript/lang/VariableString.java @@ -24,8 +24,8 @@ import com.google.common.collect.Lists; import org.bukkit.ChatColor; import org.bukkit.event.Event; -import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.skriptlang.skript.lang.script.Script; import java.util.ArrayList; @@ -385,6 +385,19 @@ public String toUnformattedString(Event event) { * @return Message components. */ public List getMessageComponents(Event event) { + return getMessageComponents(event, null); + } + + /** + * Gets message components from this string. Formatting is parsed only + * in simple parts for security reasons. Providing a StringBuilder allows an unformatted output + * identical to {@link #toUnformattedString(Event)} while only evaluating any expressions once. + * + * @param event Currently running event. + * @param unformattedBuilder Unformatted string to append to. + * @return Message components. + */ + public List getMessageComponents(Event event, @Nullable StringBuilder unformattedBuilder) { if (isSimple) { // Trusted, constant string in a script assert simpleUnformatted != null; return ChatMessages.parse(simpleUnformatted); @@ -408,14 +421,15 @@ public List getMessageComponents(Event event) { // Convert it to plain text String text = null; - if (string instanceof ExprColoured && ((ExprColoured) string).isUnsafeFormat()) { // Special case: user wants to process formatting - String unformatted = Classes.toString(((ExprColoured) string).getArray(event), true, mode); - if (unformatted != null) { - message.addAll(ChatMessages.parse(unformatted)); + if (string instanceof Expression expression) { + text = Classes.toString(expression.getArray(event), true, mode); + if (unformattedBuilder != null) + unformattedBuilder.append(text); + // Special case: user wants to process formatting + if (string instanceof ExprColoured exprColoured && exprColoured.isUnsafeFormat()) { + message.addAll(ChatMessages.parse(text)); + continue; } - continue; - } else if (string instanceof Expression) { - text = Classes.toString(((Expression) string).getArray(event), true, mode); } assert text != null; diff --git a/src/test/skript/tests/regressions/7718-broadcast evaluates vstrings twice.sk b/src/test/skript/tests/regressions/7718-broadcast evaluates vstrings twice.sk new file mode 100644 index 00000000000..d3a20ddae84 --- /dev/null +++ b/src/test/skript/tests/regressions/7718-broadcast evaluates vstrings twice.sk @@ -0,0 +1,8 @@ +test "broadcast evaluates vstrings twice": + broadcast "%func()%" + assert {7718::calls} is 1 with "Called func() more than once" + delete {7718::calls} + +function func() returns string: + add 1 to {7718::calls} + return "7718 broadcast regression test"