You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Philzen (via GitHub)" <gi...@apache.org> on 2024/01/04 10:25:18 UTC
[PR] Amend description to reflect empty string result for windows drive [commons-io]
Philzen opened a new pull request, #552:
URL: https://github.com/apache/commons-io/pull/552
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:
```java
@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:
```java
@Test void returnsHomeWithTrailingSlash_forHomeWithTrailingSlash() {
assertThat(FilenameUtils.getFullPathNoEndSeparator("C:")).isEqualTo("");
}
```
This PR proposes to reflect this change in the javadoc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Amend description to reflect empty string result for windows drive [commons-io]
Posted by "govind-gupta-tfs (via GitHub)" <gi...@apache.org>.
govind-gupta-tfs commented on PR #552:
URL: https://github.com/apache/commons-io/pull/552#issuecomment-3380795685
`
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Amend description to reflect empty string result for windows drive [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #552:
URL: https://github.com/apache/commons-io/pull/552#issuecomment-3383436374
Hi @govind-gupta-tfs,
Apache Commons IO already provides utilities to guard against path-traversal and invalid names:
* [`FilenameUtils.getName(...)`](https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/FilenameUtils.html#getName%28java.lang.String%29): strips any path components from user input, preventing multiple segments (e.g., `../../evil.txt` → `evil.txt`).
* [`FileSystem.isLegalFileName(...)`](https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/FileSystem.html#isLegalFileName%28java.lang.CharSequence%29): lets you detect illegal characters and reserved file names for the current platform.
* [`FileSystem.toLegalFileName(...)`](https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/FileSystem.html#toLegalFileName%28java.lang.String,char%29): 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Amend description to reflect empty string result for windows drive [commons-io]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #552:
URL: https://github.com/apache/commons-io/pull/552#issuecomment-1878704098
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Amend description to reflect empty string result for windows drive [commons-io]
Posted by "Philzen (via GitHub)" <gi...@apache.org>.
Philzen commented on PR #552:
URL: https://github.com/apache/commons-io/pull/552#issuecomment-2074987853
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Amend description to reflect empty string result for windows drive [commons-io]
Posted by "govind-gupta-tfs (via GitHub)" <gi...@apache.org>.
govind-gupta-tfs commented on PR #552:
URL: https://github.com/apache/commons-io/pull/552#issuecomment-3380799748
@elharo pls see the above message
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org