Skip to content

Commit 7e302f9

Browse files
committed
log error if script path contains illegal characters
1 parent 1ad2df4 commit 7e302f9

File tree

5 files changed

+72
-15
lines changed

5 files changed

+72
-15
lines changed

src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledClass.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class CompiledClass {
1616
public static final String CLASS_SUFFIX = ".clz";
1717

1818
final String path;
19-
String name;
19+
final String name;
2020
byte[] data;
2121
Class<?> clazz;
2222

src/main/java/com/cleanroommc/groovyscript/sandbox/CustomGroovyScriptEngine.java

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,13 @@ List<CompiledScript> findScripts(Collection<File> files) {
184184
List<CompiledScript> scripts = new ArrayList<>(files.size());
185185
for (File file : files) {
186186
CompiledScript cs = checkScriptLoadability(file);
187-
if (!cs.preprocessorCheckFailed()) scripts.add(cs);
187+
if (cs != null && !cs.preprocessorCheckFailed()) scripts.add(cs);
188188
}
189189
return scripts;
190190
}
191191

192192
void loadScript(CompiledScript script) {
193-
if (script.requiresReload() && !script.preprocessorCheckFailed()) {
193+
if (script != null && script.requiresReload() && !script.preprocessorCheckFailed()) {
194194
Class<?> clazz = loadScriptClassInternal(new File(script.path), true);
195195
script.setRequiresReload(false);
196196
if (script.clazz == null) {
@@ -207,10 +207,13 @@ CompiledScript loadScriptClass(File file) {
207207
return compiledScript;
208208
}
209209

210-
@NotNull
211210
CompiledScript checkScriptLoadability(File file) {
212211
String relativeFileName = FileUtil.relativize(this.scriptRoot.getPath(), file.getPath());
213-
File relativeFile = new File(relativeFileName);
212+
if (!FileUtil.validateScriptPath(relativeFileName)) {
213+
// skip
214+
return null;
215+
}
216+
// File relativeFile = new File(relativeFileName);
214217
long lastModified = file.lastModified();
215218
CompiledScript comp = this.index.get(relativeFileName);
216219

@@ -294,8 +297,11 @@ private File findScriptFile(String scriptName) {
294297

295298
private @Nullable Class<?> parseDynamicScript(File file, boolean isFileRelative) {
296299
if (isFileRelative) {
297-
file = findScriptFile(file.getPath());
298-
if (file == null) return null;
300+
File absoutefile = findScriptFile(file.getPath());
301+
if (absoutefile == null) {
302+
throw new IllegalArgumentException("Now file was found for '" + file.getPath() + "'");
303+
}
304+
file = absoutefile;
299305
}
300306
Class<?> clazz = null;
301307
try {
@@ -488,14 +494,16 @@ public LookupResult findClassNode(String origName, CompilationUnit compilationUn
488494
File scriptFile = CustomGroovyScriptEngine.this.findScriptFileOfClass(name);
489495
if (scriptFile != null) {
490496
CompiledScript result = checkScriptLoadability(scriptFile);
491-
if (result.requiresReload() || result.clazz == null) {
492-
try {
493-
return new LookupResult(compilationUnit.addSource(scriptFile.toURI().toURL()), null);
494-
} catch (MalformedURLException e) {
495-
throw new RuntimeException(e);
497+
if (result != null) {
498+
if (result.requiresReload() || result.clazz == null) {
499+
try {
500+
return new LookupResult(compilationUnit.addSource(scriptFile.toURI().toURL()), null);
501+
} catch (MalformedURLException e) {
502+
throw new RuntimeException(e);
503+
}
504+
} else {
505+
return new LookupResult(null, ClassHelper.make(result.clazz));
496506
}
497-
} else {
498-
return new LookupResult(null, ClassHelper.make(result.clazz));
499507
}
500508
}
501509
return super.findClassNode(origName, compilationUnit);

src/main/java/com/cleanroommc/groovyscript/sandbox/FileUtil.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.cleanroommc.groovyscript.sandbox;
22

3+
import com.cleanroommc.groovyscript.api.GroovyLog;
34
import it.unimi.dsi.fastutil.chars.Char2ObjectOpenHashMap;
5+
import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet;
46
import org.apache.commons.lang3.StringUtils;
57
import org.jetbrains.annotations.Contract;
68
import org.jetbrains.annotations.NotNull;
@@ -12,10 +14,14 @@
1214
import java.nio.charset.Charset;
1315
import java.nio.charset.StandardCharsets;
1416
import java.nio.file.Files;
17+
import java.util.Set;
18+
import java.util.regex.Pattern;
1519

1620
public class FileUtil {
1721

1822
private static final Char2ObjectOpenHashMap<String> encodings = new Char2ObjectOpenHashMap<>();
23+
private static final Set<String> warnedAboutPackage = new ObjectOpenHashSet<>();
24+
private static final Pattern pathPattern = Pattern.compile("^[a-zA-Z][a-zA-Z0-9_]*$");
1925

2026
static {
2127
encodings.put(' ', "%20");
@@ -47,6 +53,48 @@ public class FileUtil {
4753
}
4854
}
4955

56+
public static void cleanScriptPathWarnedCache() {
57+
warnedAboutPackage.clear();
58+
}
59+
60+
/**
61+
* Logs an error for every directory in the path if it contains illegal characters.
62+
*
63+
* @param path relative script path to check for
64+
* @return if script path is valid
65+
*/
66+
public static boolean validateScriptPath(String path) {
67+
int index = path.lastIndexOf('.');
68+
if (index < 0) {
69+
GroovyLog.get().errorMC("Script path '{}' doesn't have a file ending.", decodeURI(path));
70+
return false;
71+
}
72+
String[] parts = path.substring(0, index).split("/");
73+
boolean errorInFile = false;
74+
boolean errorInPath = false;
75+
for (int i = 0; i < parts.length; i++) {
76+
String part = parts[i];
77+
if (!pathPattern.matcher(part).matches()) {
78+
String fullPath = StringUtils.join(parts, '/', 0, i + 1);
79+
if (i == parts.length - 1) {
80+
errorInFile = true;
81+
} else {
82+
errorInPath = true;
83+
if (!warnedAboutPackage.contains(fullPath)) {
84+
warnedAboutPackage.add(fullPath);
85+
GroovyLog.get().errorMC("Script path '{}' contains illegal characters. Only letters, numbers and underscores are allowed for folder and file names. Folders and files must start with a letter!", decodeURI(fullPath));
86+
}
87+
}
88+
}
89+
}
90+
if (errorInFile) {
91+
GroovyLog.get().errorMC("Skipping script '{}', because path contains illegal characters. Only letters, numbers and underscores are allowed for folder and file names. Folders and files must start with a letter!", decodeURI(path));
92+
} else if (errorInPath) {
93+
GroovyLog.get().error("Skipping script '{}' because of illegal characters", decodeURI(path));
94+
}
95+
return !errorInFile && !errorInPath;
96+
}
97+
5098
public static @NotNull String relativize(String rootPath, String longerThanRootPath) {
5199
longerThanRootPath = encodeURI(fixPath(decodeURI(longerThanRootPath)));
52100
rootPath = encodeURI(rootPath);

src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyScriptSandbox.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ protected void load(Binding binding, Set<String> executedClasses, boolean run) {
234234
}
235235

236236
protected void loadScripts(Binding binding, Set<String> executedClasses, boolean run) {
237+
FileUtil.cleanScriptPathWarnedCache();
237238
for (CompiledScript compiledScript : this.engine.findScripts(getScriptFiles())) {
238239
if (!executedClasses.contains(compiledScript.path)) {
239240
long t = System.currentTimeMillis();

src/main/java/com/cleanroommc/groovyscript/sandbox/RunConfig.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ public boolean isLoaderConfigured(String loader) {
259259
public Collection<File> getSortedFiles(File root, String loader) {
260260
List<String> paths = loaderPaths.get(loader);
261261
if (paths == null || paths.isEmpty()) return Collections.emptyList();
262-
return SandboxData.getSortedFilesOf(root, paths);
262+
return SandboxData.getSortedFilesOf(root, paths, isDebug());
263263
}
264264

265265
private static String sanitizePath(String path) {

0 commit comments

Comments
 (0)