Amend description to reflect empty string result for windows drive
Not sure if this is is a bug or a feature – what i can confirm is that in v2.4 it worked as it said in the description:
@Test void returnsHomeWithTrailingSlash_forHomeWithTrailingSlash() {
assertThat(FilenameUtils.getFullPathNoEndSeparator("C:")).isEqualTo("C:");
}
After upgrading to 2.15.1, i realized my test suite failed, the behaviour is now:
@Test void returnsHomeWithTrailingSlash_forHomeWithTrailingSlash() {
assertThat(FilenameUtils.getFullPathNoEndSeparator("C:")).isEqualTo("");
}
This PR proposes to reflect this change in the javadoc.
You've pointed to existing bad API design. getFullPathNoEndSeparator("C:") should return "" if the argument is a Unix path and "C:" if that path is a Windows path. Of course, it's just a string, and we have no way of knowing whether that's a Windows path or a Posix path. In this case, best guess is Windows since "C:" is very common in Windows and very uncommon on Unix, but it's still a guess. Possibly we should deprecate this method and rethink the API here.
Possibly we should deprecate this method and rethink the API here.
@elharo Agreed. For the time being, any objection merging this PR to bring the JavaDoc inline with the method's actual behavior?
`
import org.apache.commons.lang3.Validate; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.util.Locale; import java.util.Set;
public class PathValidator {
private static final Set<String> RESERVED_NAMES = Set.of(
"CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6",
"COM7", "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6",
"LPT7", "LPT8", "LPT9"
);
public static void validateSafePathComponent(String component) {
// --- 1. Basic Checks (Using Apache Commons Lang) ---
Validate.notBlank(component, "Path component must not be blank");
Validate.isTrue(component.length() <= 255, "Path component too long");
// Pre-decode raw dangerous percent encodings
String lower = component.toLowerCase(Locale.ROOT);
if (lower.contains("%2f") || lower.contains("%5c") || lower.contains("%00")) {
throw new IllegalArgumentException("Encoded separator/null not allowed: " + component);
}
// --- 2. Decoding ---
String decoded;
try {
decoded = URLDecoder.decode(component, StandardCharsets.UTF_8);
} catch (IllegalArgumentException ex) {
throw new IllegalArgumentException("Invalid percent encoding: " + component);
}
// --- 3. Path Traversal/Normalization Check (Using Path API) ---
Path p;
try {
p = Path.of(decoded);
} catch (InvalidPathException e) {
throw new IllegalArgumentException("Invalid path syntax: " + component);
}
// Critical Path Traversal check: must be a single name and not contain '..' or '.'
if (p.isAbsolute() || p.getNameCount() != 1 || !p.normalize().equals(p)) {
throw new IllegalArgumentException("Traversal/dot segments or absolute path not allowed: " + component);
}
String name = p.getFileName().toString();
// --- 4. Final Name/Custom Policy Checks ---
// Basic character / separator checks
if (name.contains("/") || name.contains("\\") || name.indexOf('\0') >= 0 || name.contains("+")) {
throw new IllegalArgumentException("Unsafe characters (separators/null/plus): " + component);
}
// Control chars
for (int i = 0; i < name.length(); i++) {
char c = name.charAt(i);
if (c < 0x20 || c == 0x7F) {
throw new IllegalArgumentException("Control character present: " + component);
}
}
// Dot-only or reserved dot forms & Trailing space/dot
if (name.equals(".") || name.equals("..") || name.matches("\\.{2,}") ||
name.endsWith(" ") || name.endsWith(".")) {
throw new IllegalArgumentException("Invalid dot sequence or trailing space/dot: " + component);
}
// Allowlist
if (!name.matches("[A-Za-z0-9._-]+")) {
throw new IllegalArgumentException("Disallowed characters: " + component);
}
// Windows reserved device names
if (RESERVED_NAMES.contains(name.toUpperCase(Locale.ROOT))) {
throw new IllegalArgumentException("Reserved device name: " + component);
}
}
}`
@garydgregory can we make some code in apache commons to avoid path-injection attacks like the code above for aavoiding path injections in windows?
@elharo pls see the above message
Hi @govind-gupta-tfs,
Apache Commons IO already provides utilities to guard against path-traversal and invalid names:
FilenameUtils.getName(...): strips any path components from user input, preventing multiple segments (e.g.,../../evil.txt→evil.txt).FileSystem.isLegalFileName(...): lets you detect illegal characters and reserved file names for the current platform.FileSystem.toLegalFileName(...): normalizes a user-supplied name to a safe form.
One note: toLegalFileName(...) doesn’t currently replace Windows reserved device names (e.g., CON, NUL, PRN). If you think that would help here, we’d welcome a contribution adding that behavior.