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

Handle invalid regex pattern in ExprJoinSplit #7202

Merged
merged 4 commits into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 49 additions & 18 deletions src/main/java/ch/njol/skript/expressions/ExprJoinSplit.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
package ch.njol.skript.expressions;

import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;

import ch.njol.skript.SkriptConfig;
import ch.njol.skript.lang.Literal;
import org.bukkit.event.Event;
import org.jetbrains.annotations.Nullable;

Expand All @@ -36,14 +38,11 @@
import ch.njol.util.Kleenean;
import ch.njol.util.StringUtils;

/**
* @author Peter Güttinger
*/
@Name("Join & Split")
@Description("Joins several texts with a common delimiter (e.g. \", \"), or splits a text into multiple texts at a given delimiter.")
@Examples({
"message \"Online players: %join all players with \"\" | \"\"%\" # %all players% would use the default \"x, y, and z\"",
"set {_s::*} to the string argument split at \",\""
"message \"Online players: %join all players' names with \"\" | \"\"%\" # %all players% would use the default \"x, y, and z\"",
"set {_s::*} to the string argument split at \",\""
})
@Since("2.1, 2.5.2 (regex support), 2.7 (case sensitivity)")
public class ExprJoinSplit extends SimpleExpression<String> {
Expand All @@ -61,33 +60,47 @@ public class ExprJoinSplit extends SimpleExpression<String> {
private boolean regex;
private boolean caseSensitivity;

@SuppressWarnings("null")
private Expression<String> strings;
@Nullable
private Expression<String> delimiter;
private @Nullable Expression<String> delimiter;

private @Nullable Pattern pattern;

@Override
@SuppressWarnings({"unchecked", "null"})
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
join = matchedPattern == 0;
regex = matchedPattern >= 3;
caseSensitivity = SkriptConfig.caseSensitive.value() || parseResult.hasTag("case");
//noinspection unchecked
strings = (Expression<String>) exprs[0];
//noinspection unchecked
delimiter = (Expression<String>) exprs[1];
if (!join && delimiter instanceof Literal) {
String stringPattern = ((Literal<String>) delimiter).getSingle();
try {
this.pattern = compilePattern(stringPattern);
} catch (PatternSyntaxException e) {
Skript.error("'" + stringPattern + "' is not a valid regular expression");
return false;
}
}
return true;
}

@Override
@Nullable
protected String[] get(Event event) {
protected String @Nullable [] get(Event event) {
String[] strings = this.strings.getArray(event);
String delimiter = this.delimiter != null ? this.delimiter.getSingle(event) : "";
if (strings.length == 0 || delimiter == null)
return new String[0];
if (join) {
if (join)
return new String[] {StringUtils.join(strings, delimiter)};
} else {
return strings[0].split(regex ? delimiter : (caseSensitivity ? "" : "(?i)") + Pattern.quote(delimiter), -1);
try {
Pattern pattern = this.pattern;
if (pattern == null)
pattern = compilePattern(delimiter);
return pattern.split(strings[0], -1);
} catch (PatternSyntaxException e) {
return new String[0];
}
}

Expand All @@ -103,10 +116,28 @@ public Class<? extends String> getReturnType() {

@Override
public String toString(@Nullable Event event, boolean debug) {
if (join)
return "join " + strings.toString(event, debug) + (delimiter != null ? " with " + delimiter.toString(event, debug) : "");
return (regex ? "regex " : "") + "split " + strings.toString(event, debug) + (delimiter != null ? " at " + delimiter.toString(event, debug) : "")
+ (regex ? "" : "(case sensitive: " + caseSensitivity + ")");
StringBuilder builder = new StringBuilder();
if (join) {
builder.append("join ").append(strings.toString(event, debug));
if (delimiter != null)
builder.append(" with ").append(delimiter.toString(event, debug));
return builder.toString();
}

assert delimiter != null;
if (regex)
builder.append("regex ");
builder.append("split ")
.append(strings.toString(event, debug))
.append(" at ")
.append(delimiter.toString(event, debug));
if (!regex)
builder.append(" (case sensitive: ").append(caseSensitivity).append(")");
return builder.toString();
}

private Pattern compilePattern(String delimiter) {
return Pattern.compile(regex ? delimiter : (caseSensitivity ? "" : "(?i)") + Pattern.quote(delimiter));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
test "regex exceptions not handled":
parse:
set {_split::*} to regex split "test" at "\b{_name}\b"
assert last parse logs is "'\b{_name}\b' is not a valid regular expression" with "regex split did not error with invalid regex literal"

set {_pattern} to "\b{_name}\b"
set {_split::*} to regex split "test" at {_pattern}
assert {_split::*} is not set with "regex split returned a value with invalid regex expression"

assert regex split "apple,banana;cherry" at "[,;]" is "apple", "banana" and "cherry" with "regex split did not split correctly with literal"
set {_pattern} to "[,;]"
assert regex split "apple,banana;cherry" at {_pattern} is "apple", "banana" and "cherry" with "regex split did not split correctly with expression"