Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Amend description to reflect empty string result for windows drive#552

Open
Philzen wants to merge 1 commit intoapache:master from
Philzen:patch-2
Open

Amend description to reflect empty string result for windows drive #552
Philzen wants to merge 1 commit intoapache:master from
Philzen:patch-2

Conversation

@Philzen
Copy link

@Philzen Philzen commented Jan 4, 2024

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.

Copy link
Contributor

elharo commented Jan 5, 2024

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.

Copy link
Author

Philzen commented Apr 24, 2024

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?

Copy link

`

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?

Copy link

@elharo pls see the above message

Copy link
Contributor

Hi @govind-gupta-tfs,

Apache Commons IO already provides utilities to guard against path-traversal and invalid names:

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /