Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minimessage: Ignore invalid tags when parsing #723

Merged
merged 5 commits into from Mar 5, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -48,12 +48,24 @@ private TagInternals() {
* @param tagName the name of the tag
* @since 4.10.0
*/
public static void checkTagName(final @NotNull String tagName) {
public static void assertValidTagName(final @NotNull String tagName) {
if (!TAG_NAME_PATTERN.matcher(Objects.requireNonNull(tagName)).matches()) {
throw new IllegalArgumentException("Tag name must match pattern " + TAG_NAME_PATTERN.pattern() + ", was " + tagName);
}
}

/**
* Checks if a tag name matches the pattern for allowed tag names, first sanitizing it
* by converting the tag name to lowercase. Returns a boolean representing the validity
*
* @param tagName the name of the tag
* @return validity of this tag when sanitized
* @since 4.10.1
*/
public static boolean sanitizeAndCheckValidTagName(final @NotNull String tagName) {
return TAG_NAME_PATTERN.matcher(Objects.requireNonNull(tagName).toLowerCase(Locale.ROOT)).matches();
}

/**
* Checks if a tag name matches the pattern for allowed tag names, first sanitizing it
* by converting the tag name to lowercase. If it does not match the pattern, then this
Expand All @@ -62,7 +74,7 @@ public static void checkTagName(final @NotNull String tagName) {
* @param tagName the name of the tag
* @since 4.10.0
*/
public static void sanitizeAndCheckTagName(final @NotNull String tagName) {
checkTagName(Objects.requireNonNull(tagName).toLowerCase(Locale.ROOT));
public static void sanitizeAndAssertValidTagName(final @NotNull String tagName) {
assertValidTagName(Objects.requireNonNull(tagName).toLowerCase(Locale.ROOT));
}
}
Expand Up @@ -31,6 +31,7 @@
import java.util.function.IntPredicate;
import java.util.function.Predicate;
import net.kyori.adventure.text.minimessage.ParsingException;
import net.kyori.adventure.text.minimessage.internal.TagInternals;
import net.kyori.adventure.text.minimessage.internal.parser.match.MatchedTokenConsumer;
import net.kyori.adventure.text.minimessage.internal.parser.match.StringResolvingMatchedTokenConsumer;
import net.kyori.adventure.text.minimessage.internal.parser.match.TokenListProducingMatchedTokenConsumer;
Expand Down Expand Up @@ -375,6 +376,15 @@ private static RootNode buildTree(

case OPEN_TAG:
case OPEN_CLOSE_TAG:
// Check if this even is a valid tag
final Token tagNamePart = token.childTokens().get(0);
final String tagName = message.substring(tagNamePart.startIndex(), tagNamePart.endIndex());
if (!TagInternals.sanitizeAndCheckValidTagName(tagName)) {
// This wouldn't be a valid tag, just parse it as text instead!
node.addChild(new TextNode(node, token, message));
break;
}

final TagNode tagNode = new TagNode(node, token, message, tagProvider);
if (tagNameChecker.test(tagNode.name())) {
final Tag tag = tagProvider.resolve(tagNode);
Expand Down
Expand Up @@ -24,6 +24,7 @@
package net.kyori.adventure.text.minimessage.internal.parser.match;

import java.util.Objects;
import net.kyori.adventure.text.minimessage.internal.TagInternals;
import net.kyori.adventure.text.minimessage.internal.parser.TokenParser;
import net.kyori.adventure.text.minimessage.internal.parser.TokenParser.TagProvider;
import net.kyori.adventure.text.minimessage.internal.parser.TokenType;
Expand Down Expand Up @@ -69,12 +70,15 @@ public void accept(final int start, final int end, final @NotNull TokenType toke
final String match = this.input.substring(start, end);
final String tag = this.input.substring(start + 1, end - 1);

// we might care if it's a pre-process!
final @Nullable Tag replacement = this.tagProvider.resolve(TokenParser.TagProvider.sanitizePlaceholderName(tag));
// we might care if it's a valid tag!
if (TagInternals.sanitizeAndCheckValidTagName(tag)) {
// we might care if it's a pre-process!
final @Nullable Tag replacement = this.tagProvider.resolve(TokenParser.TagProvider.sanitizePlaceholderName(tag));

if (replacement instanceof PreProcess) {
this.builder.append(Objects.requireNonNull(((PreProcess) replacement).value(), "PreProcess replacements cannot return null"));
return;
if (replacement instanceof PreProcess) {
this.builder.append(Objects.requireNonNull(((PreProcess) replacement).value(), "PreProcess replacements cannot return null"));
return;
}
}

// if we get here, the placeholder wasn't found or was null
Expand Down
Expand Up @@ -68,7 +68,7 @@ public TagNode(

// Then assert the tag node has a proper name.
try {
TagInternals.sanitizeAndCheckTagName(this.name());
TagInternals.sanitizeAndAssertValidTagName(this.name());
rymiel marked this conversation as resolved.
Show resolved Hide resolved
} catch (final IllegalArgumentException | NullPointerException e) {
throw new ParsingExceptionImpl("Invalid tag name " + this.name(), this.sourceMessage(), e, this.token());
}
Expand Down
Expand Up @@ -71,7 +71,7 @@ public interface SerializableResolver {
static @NotNull TagResolver claimingComponent(final @NotNull Set<String> names, final @NotNull BiFunction<ArgumentQueue, Context, Tag> handler, final @NotNull Function<Component, @Nullable Emitable> componentClaim) {
final Set<String> ownNames = new HashSet<>(names);
for (final String name : ownNames) {
TagInternals.checkTagName(name);
TagInternals.assertValidTagName(name);
}
requireNonNull(handler, "handler");
return new ComponentClaimingResolverImpl(ownNames, handler, componentClaim);
Expand Down Expand Up @@ -102,7 +102,7 @@ public interface SerializableResolver {
static @NotNull TagResolver claimingStyle(final @NotNull Set<String> names, final @NotNull BiFunction<ArgumentQueue, Context, Tag> handler, final @NotNull StyleClaim<?> styleClaim) {
final Set<String> ownNames = new HashSet<>(names);
for (final String name : ownNames) {
TagInternals.checkTagName(name);
TagInternals.assertValidTagName(name);
}
requireNonNull(handler, "handler");
return new StyleClaimingResolverImpl(ownNames, handler, styleClaim);
Expand Down
Expand Up @@ -92,7 +92,7 @@ public interface TagResolver {
* @since 4.10.0
*/
static TagResolver.@NotNull Single resolver(final @NotNull String name, final @NotNull Tag tag) {
TagInternals.checkTagName(name);
TagInternals.assertValidTagName(name);
return new SingleResolver(
name,
requireNonNull(tag, "tag")
Expand Down Expand Up @@ -122,7 +122,7 @@ public interface TagResolver {
static @NotNull TagResolver resolver(final @NotNull Set<String> names, final @NotNull BiFunction<ArgumentQueue, Context, Tag> handler) {
final Set<String> ownNames = new HashSet<>(names);
for (final String name : ownNames) {
TagInternals.checkTagName(name);
TagInternals.assertValidTagName(name);
}
requireNonNull(handler, "handler");

Expand Down
Expand Up @@ -26,10 +26,13 @@
import net.kyori.adventure.text.Component;
import net.kyori.adventure.text.TextComponent;
import net.kyori.adventure.text.format.NamedTextColor;
import net.kyori.adventure.text.minimessage.tag.Tag;
import net.kyori.adventure.text.minimessage.tag.resolver.ArgumentQueue;
import net.kyori.adventure.text.minimessage.tag.resolver.Placeholder;
import net.kyori.adventure.text.minimessage.tag.resolver.TagResolver;
import net.kyori.adventure.text.minimessage.tree.Node;
import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;

import static net.kyori.adventure.text.Component.empty;
Expand Down Expand Up @@ -443,15 +446,19 @@ void testLegacySymbolForbidden() {

@Test
void testInvalidTagNames() {
final String failingTest = "Hello <this_is_%not_allowed> but cool?";
final String failingTest1 = "Hello <this_is_not_allowed!> but cool?";
final String failingTest2 = "Hello <!?this_is_not_allowed> but cool?";
final String failingTest3 = "Hello <##this_is_%not_allowed> but cool?";
final String input1 = "Hello <this_is_%not_allowed> but ignored?";
final String input2 = "Hello <this_is_not_allowed!> but ignored?";
final String input3 = "Hello <!?this_is_not_allowed> but ignored?";
final String input4 = "Hello <##this_is_%not_allowed> but ignored?";
final String input5 = "<3 >Mini<3 />Message</3 >";
final String input6 = "this message <\"red\">isn't red";

assertThrows(ParsingException.class, () -> PARSER.deserialize(failingTest));
assertThrows(ParsingException.class, () -> PARSER.deserialize(failingTest1));
assertThrows(ParsingException.class, () -> PARSER.deserialize(failingTest2));
assertThrows(ParsingException.class, () -> PARSER.deserialize(failingTest3));
this.assertParsedEquals(Component.text(input1), input1);
this.assertParsedEquals(Component.text(input2), input2);
this.assertParsedEquals(Component.text(input3), input3);
this.assertParsedEquals(Component.text(input4), input4);
this.assertParsedEquals(Component.text(input5), input5);
this.assertParsedEquals(Component.text(input6), input6);
}

@Test
Expand All @@ -468,4 +475,23 @@ void testValidTagNames() {
assertDoesNotThrow(() -> PARSER.deserialize(passingTest3));
assertDoesNotThrow(() -> PARSER.deserialize(passingTest4));
}

@Test
void invalidPreprocessTagNames() {
final String input = "Some<##>of<>these<tag>are<3 >tags";
final Component expected = Component.text("Some<##>of<>these(meow)are<3 >tags");
final TagResolver alwaysMatchingResolver = new TagResolver() {
@Override
public Tag resolve(final @NotNull String name, final @NotNull ArgumentQueue arguments, final @NotNull Context ctx) throws ParsingException {
return Tag.preProcessParsed("(meow)");
}

@Override
public boolean has(final @NotNull String name) {
return true;
}
};

assertParsedEquals(expected, input, alwaysMatchingResolver);
}
}