Add limit to shorthand parser (#4293)

common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionSet.java

@@ -38,6 +38,8 @@ import me.lucko.luckperms.common.command.utils.StorageAssistant;
 import me.lucko.luckperms.common.locale.Message;
 import me.lucko.luckperms.common.model.PermissionHolder;
 import me.lucko.luckperms.common.node.factory.NodeBuilders;
+import me.lucko.luckperms.common.node.utils.ShorthandParseException;
+import me.lucko.luckperms.common.node.utils.ShorthandParser;
 import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
 import me.lucko.luckperms.common.sender.Sender;
 import me.lucko.luckperms.common.util.Predicates;
@@ -46,6 +48,7 @@ import net.luckperms.api.model.data.DataMutateResult;
 import net.luckperms.api.model.data.DataType;
 import net.luckperms.api.node.Node;
 import net.luckperms.api.node.types.InheritanceNode;
+import net.luckperms.api.node.types.PermissionNode;
 
 import java.util.List;
 
@@ -86,6 +89,13 @@ public class PermissionSet extends GenericChildCommand {
             }
         }
 
+        if (builtNode instanceof PermissionNode) {
+            ShorthandParseException shorthandError = ShorthandParser.checkParse(builtNode.getKey());
+            if (shorthandError != null) {
+                Message.SHORTHAND_PARSE_ERROR.send(sender, builtNode.getKey(), shorthandError.getMessage());
+            }
+        }
+
         DataMutateResult result = target.setNode(DataType.NORMAL, builtNode, true);
 
         if (result.wasSuccessful()) {

common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionSetTemp.java

@@ -39,6 +39,8 @@ import me.lucko.luckperms.common.config.ConfigKeys;
 import me.lucko.luckperms.common.locale.Message;
 import me.lucko.luckperms.common.model.PermissionHolder;
 import me.lucko.luckperms.common.node.factory.NodeBuilders;
+import me.lucko.luckperms.common.node.utils.ShorthandParseException;
+import me.lucko.luckperms.common.node.utils.ShorthandParser;
 import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
 import me.lucko.luckperms.common.sender.Sender;
 import me.lucko.luckperms.common.util.Predicates;
@@ -48,6 +50,7 @@ import net.luckperms.api.model.data.DataType;
 import net.luckperms.api.model.data.TemporaryNodeMergeStrategy;
 import net.luckperms.api.node.Node;
 import net.luckperms.api.node.types.InheritanceNode;
+import net.luckperms.api.node.types.PermissionNode;
 
 import java.time.Duration;
 import java.util.List;
@@ -91,6 +94,13 @@ public class PermissionSetTemp extends GenericChildCommand {
             }
         }
 
+        if (builtNode instanceof PermissionNode) {
+            ShorthandParseException shorthandError = ShorthandParser.checkParse(builtNode.getKey());
+            if (shorthandError != null) {
+                Message.SHORTHAND_PARSE_ERROR.send(sender, builtNode.getKey(), shorthandError.getMessage());
+            }
+        }
+
         DataMutateResult.WithMergedNode result = target.setNode(DataType.NORMAL, builtNode, modifier);
 
         if (result.getResult().wasSuccessful()) {

common/src/main/java/me/lucko/luckperms/common/locale/Message.java

@@ -2283,6 +2283,16 @@ public interface Message {
     Args0 INVALID_META_KEY_EMPTY = () -> INVALID_INPUT_EMPTY.build("meta-key");
     Args0 INVALID_DISPLAY_NAME_EMPTY = () -> INVALID_INPUT_EMPTY.build("display-name");
 
+    Args2<String, String> SHORTHAND_PARSE_ERROR = (key, error) -> prefixed(translatable()
+            // "&cWarning: Permission &f{}&c could not be parsed as shorthand: &4{}&c"
+            .key("luckperms.command.misc.shorthand-parse-error")
+            .color(RED)
+            .args(
+                    text(key, WHITE),
+                    text(error, DARK_RED)
+            )
+    );
+
     Args3<PermissionHolder, Group, ContextSet> SET_INHERIT_SUCCESS = (holder, parent, context) -> prefixed(translatable()
             // "&b{}&a now inherits permissions from &b{}&a in context {}&a."
             .key("luckperms.command.generic.parent.add")

common/src/main/java/me/lucko/luckperms/common/node/AbstractNode.java

@@ -73,7 +73,9 @@ public abstract class AbstractNode<N extends ScopedNode<N, B>, B extends NodeBui
         this.contexts = contexts;
         this.metadata = ImmutableMap.copyOf(metadata);
 
-        this.resolvedShorthand = this instanceof PermissionNode ? ImmutableList.copyOf(ShorthandParser.expandShorthand(this.key)) : ImmutableList.of();
+        this.resolvedShorthand = this instanceof PermissionNode
+                ? ImmutableList.copyOf(ShorthandParser.expandShorthandSafely(this.key))
+                : ImmutableList.of();
 
         this.hashCode = calculateHashCode();
     }

common/src/main/java/me/lucko/luckperms/common/node/utils/ShorthandParseException.java

@@ -0,0 +1,32 @@
+/*
+ * This file is part of LuckPerms, licensed under the MIT License.
+ *
+ *  Copyright (c) lucko (Luck) <luck@lucko.me>
+ *  Copyright (c) contributors
+ *
+ *  Permission is hereby granted, free of charge, to any person obtaining a copy
+ *  of this software and associated documentation files (the "Software"), to deal
+ *  in the Software without restriction, including without limitation the rights
+ *  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ *  copies of the Software, and to permit persons to whom the Software is
+ *  furnished to do so, subject to the following conditions:
+ *
+ *  The above copyright notice and this permission notice shall be included in all
+ *  copies or substantial portions of the Software.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ *  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ *  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ *  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ *  SOFTWARE.
+ */
+
+package me.lucko.luckperms.common.node.utils;
+
+public class ShorthandParseException extends Exception {
+    public ShorthandParseException(String msg) {
+        super(msg);
+    }
+}

common/src/main/java/me/lucko/luckperms/common/node/utils/ShorthandParser.java

@@ -29,6 +29,7 @@ import com.google.common.base.CharMatcher;
 import com.google.common.base.Splitter;
 import com.google.common.collect.Iterators;
 
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
@@ -43,13 +44,21 @@ public enum ShorthandParser {
      */
     NUMERIC_RANGE {
         @Override
-        public Iterator<String> extract(String input) throws NumberFormatException {
+        public Iterator<String> extract(String input) throws ShorthandParseException {
             int index = input.indexOf(RANGE_SEPARATOR);
             if (index == -1 || index == 0 || index == input.length() - 1) {
                 return null;
             }
 
-            return new RangeIterator(Integer.parseInt(input.substring(0, index)), Integer.parseInt(input.substring(index + 1))) {
+            int a, b;
+            try {
+                a = Integer.parseInt(input.substring(0, index));
+                b = Integer.parseInt(input.substring(index + 1));
+            } catch (NumberFormatException e) {
+                return null;
+            }
+
+            return new RangeIterator(a, b, false) {
                 @Override
                 protected String toString(int i) {
                     return Integer.toString(i);
@@ -63,12 +72,12 @@ public enum ShorthandParser {
      */
     CHARACTER_RANGE {
         @Override
-        public Iterator<String> extract(String input) {
+        public Iterator<String> extract(String input) throws ShorthandParseException {
             if (input.length() != 3 || input.charAt(1) != RANGE_SEPARATOR) {
                 return null;
             }
 
-            return new RangeIterator(input.charAt(0), input.charAt(2)) {
+            return new RangeIterator(input.charAt(0), input.charAt(2), true) {
                 @Override
                 protected String toString(int i) {
                     return Character.toString((char) i);
@@ -97,9 +106,9 @@ public enum ShorthandParser {
      *
      * @param input the input shorthand
      * @return an iterator of the resultant strings, or optionally null if the input was invalid
-     * @throws IllegalArgumentException if the input was invalid
+     * @throws ShorthandParseException if the range is too large or the input is invalid
      */
-    abstract Iterator<String> extract(String input) throws IllegalArgumentException;
+    abstract Iterator<String> extract(String input) throws ShorthandParseException;
 
     /** Character used to open a group */
     private static final char OPEN_GROUP = '{';
@@ -119,13 +128,46 @@ public enum ShorthandParser {
     /** The parsers */
     private static final ShorthandParser[] PARSERS = values();
 
+    /** The max number of results to return */
+    private static final int MAX_RESULTS = 1000;
+
+    /**
+     * Parses and expands the shorthand format.
+     *
+     * @param s the string to expand
+     * @return the expanded result, or an empty set if the input was invalid
+     */
+    public static Set<String> expandShorthandSafely(String s) {
+        try {
+            return expandShorthand(s);
+        } catch (ShorthandParseException e) {
+            return Collections.emptySet();
+        }
+    }
+
+    /**
+     * Check if the string can be expanded safely.
+     *
+     * @param s the string to expand
+     * @return the error, if any
+     */
+    public static ShorthandParseException checkParse(String s) {
+        try {
+            expandShorthand(s);
+            return null;
+        } catch (ShorthandParseException e) {
+            return e;
+        }
+    }
+
     /**
      * Parses and expands the shorthand format.
      *
      * @param s the string to expand
      * @return the expanded result
+     * @throws ShorthandParseException if the range is too large or the input is invalid
      */
-    public static Set<String> expandShorthand(String s) {
+    public static Set<String> expandShorthand(String s) throws ShorthandParseException {
         Set<String> results = new HashSet<>();
         results.add(s);
 
@@ -141,6 +183,10 @@ public enum ShorthandParser {
                 } else {
                     workSet.add(string);
                 }
+
+                if (workSet.size() > MAX_RESULTS) {
+                    throw new ShorthandParseException("Results exceeded limit of " + MAX_RESULTS + " when parsing '" + string + "'");
+                }
             }
 
             if (work) {
@@ -162,7 +208,7 @@ public enum ShorthandParser {
         return results;
     }
 
-    private static Set<String> matchGroup(String input) {
+    private static Set<String> matchGroup(String input) throws ShorthandParseException {
         int openingIndex = indexOfEither(input, OPEN_GROUP, OPEN_GROUP_2);
         if (openingIndex == -1) {
             return null;
@@ -215,11 +261,14 @@ public enum ShorthandParser {
         private final int max;
         private int next;
 
-        RangeIterator(int a, int b) {
+        RangeIterator(int a, int b, boolean characters) throws ShorthandParseException {
             this.max = Math.max(a, b);
             this.next = Math.min(a, b);
             if ((this.max - this.next) > MAX_RANGE) {
-                throw new IllegalArgumentException("Range too large");
+                throw new ShorthandParseException(characters
+                        ? "Range between " + (char) a + " and " + (char) b + " exceeds limit of " + MAX_RANGE
+                        : "Range between " + a + " and " + b + " exceeds limit of " + MAX_RANGE
+                );
             }
         }
 

common/src/main/resources/luckperms_en.properties

@@ -68,6 +68,7 @@ luckperms.command.misc.wait-to-finish=Please wait for it to finish and try again
 luckperms.command.misc.invalid-input-empty-permission=The empty string is not a valid permission
 luckperms.command.misc.invalid-input-empty-meta-key=The empty string is not a valid meta key
 luckperms.command.misc.invalid-input-empty-display-name=The empty string is not a valid display name
+luckperms.command.misc.shorthand-parse-error=Warning: Permission {0} could not be parsed as shorthand: {1}
 luckperms.command.misc.invalid-priority=Invalid priority {0}
 luckperms.command.misc.expected-number=Expected a number
 luckperms.command.misc.date-parse-error=Could not parse date {0}

common/src/test/java/me/lucko/luckperms/common/node/ShorthandParserTest.java

@@ -26,6 +26,7 @@
 package me.lucko.luckperms.common.node;
 
 import com.google.common.collect.ImmutableSet;
+import me.lucko.luckperms.common.node.utils.ShorthandParseException;
 import me.lucko.luckperms.common.node.utils.ShorthandParser;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -35,7 +36,7 @@ import org.junit.jupiter.params.provider.ValueSource;
 
 import java.util.stream.Stream;
 
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class ShorthandParserTest {
 
@@ -69,7 +70,7 @@ public class ShorthandParserTest {
 
     @ParameterizedTest
     @MethodSource
-    public void testParse(String shorthand, String[] expected) {
+    public void testParse(String shorthand, String[] expected) throws ShorthandParseException {
         Assertions.assertEquals(ImmutableSet.copyOf(expected), ShorthandParser.expandShorthand(shorthand));
     }
 
@@ -79,9 +80,10 @@ public class ShorthandParserTest {
             "{1000-1}",
             "{!-က}",
             "{က-!}",
+            "{1-100}{1-100}{1-100}",
     })
     public void testTooManyElements(String shorthand) {
-        assertTrue(ShorthandParser.expandShorthand(shorthand).size() <= 1);
+        assertThrows(ShorthandParseException.class, () -> ShorthandParser.expandShorthand(shorthand));
     }
 
 }

standalone/src/test/java/me/lucko/luckperms/standalone/CommandsIntegrationTest.java

@@ -254,7 +254,15 @@ public class CommandsIntegrationTest {
 
                     .givenHasPermissions("luckperms.group.permission.info")
                     .whenRunCommand("group test permission info")
-                    .thenExpect("[LP] test does not have any permissions set.");
+                    .thenExpect("[LP] test does not have any permissions set.")
+
+                    .givenHasPermissions("luckperms.group.permission.set")
+                    .whenRunCommand("group test permission set {1-1000} true")
+                    .thenExpect("""
+                            [LP] Warning: Permission {1-1000} could not be parsed as shorthand: Range between 1 and 1000 exceeds limit of 250
+                            [LP] Set {1-1000} to true for test in context global.
+                            """
+                    );
         });
     }