Skip to content

Commit

Permalink
Fixed #3786. File.dirname is munching too much off on an UNC pathname
Browse files Browse the repository at this point in the history
enebo committed Apr 7, 2016
1 parent 4752fef commit 137a315
Showing 3 changed files with 48 additions and 76 deletions.
121 changes: 47 additions & 74 deletions core/src/main/java/org/jruby/RubyFile.java
Original file line number Diff line number Diff line change
@@ -676,92 +676,65 @@ public static IRubyObject dirname(ThreadContext context, IRubyObject recv, IRuby
}

static Pattern PROTOCOL_PATTERN = Pattern.compile(URI_PREFIX_STRING + ".*");
public static String dirname(ThreadContext context, String jfilename) {
String name = jfilename.replace('\\', '/');
int minPathLength = 1;
boolean trimmedSlashes = false;

boolean startsWithDriveLetterOnWindows = startsWithDriveLetterOnWindows(name);
// get dirname with uri scheme stripped off and then recombine that back to scheme.
private static String adjustURIDirname(ThreadContext context, String path, int start) {
String adjustedPath = dirname(context, path.substring(start));
if (adjustedPath.equals(".") || adjustedPath.equals("/")) adjustedPath = "";
return path.substring(0, start) + adjustedPath;
}

if (startsWithDriveLetterOnWindows) {
minPathLength = 3;
// will strip any trailing forward or backslashes except unless the string gets too short.
private static String stripTrailingSlashes(String path, int minPathLength) {
while (path.length() > minPathLength) { // lol
int lastIndex = path.length() - 1;
char c = path.charAt(lastIndex);
if (c != '/' && c != '\\') break;
path = path.substring(0, lastIndex);

This comment has been minimized.

Copy link
@kares

kares Apr 8, 2016

Member

🍭 but there's likely to be only 1 trailing '/' or '' thus 👌

}
return path;
}
/**
* Return the dirname of the speficied filename. path is the original string. On windows
* if this is a file path it will leave the delimiter the same. If it represents a URI then it
* needs to become forward slashes.
*
* Internally we have two strings in this method: path and normalizedPath which is normalized
* to '/' so we only have to do indexes against a single format of file.
*/
// FIXME: MRI returns '//' -> '//' and '\\\\' -> '\\\\' (bug?). useless values seemingly?
public static String dirname(ThreadContext context, String path) {
String normalizedPath = path.replace('\\', '/');
boolean hasDriveLetter = startsWithDriveLetterOnWindows(normalizedPath);

// jar like paths
if (name.contains(".jar!/")) {
int start = name.indexOf("!/") + 1;
String path = dirname(context, name.substring(start));
if (path.equals(".") || path.equals("/")) path = "";
return name.substring(0, start) + path;
}
// address all the url like paths first
if (PROTOCOL_PATTERN.matcher(name).matches()) {
int start = name.indexOf(":/") + 2;
String path = dirname(context, name.substring(start));
if (path.equals(".")) path = "";
return name.substring(0, start) + path;
}
// Dealing with URIs. These are use normalized path because \ is never valid.
if (normalizedPath.contains(".jar!/")) return adjustURIDirname(context, normalizedPath, normalizedPath.indexOf("!/") + 1);
if (PROTOCOL_PATTERN.matcher(normalizedPath).matches()) return adjustURIDirname(context, normalizedPath,normalizedPath.indexOf(":/") + 2);

while (name.length() > minPathLength && name.charAt(name.length() - 1) == '/') {
trimmedSlashes = true;
name = name.substring(0, name.length() - 1);
}
if (hasDriveLetter && normalizedPath.length() == 2) return path + '.'; // 'C:' passed in

String result;
if (startsWithDriveLetterOnWindows && name.length() == 2) {
if (trimmedSlashes) {
// C:\ is returned unchanged
result = jfilename.substring(0, 3);
} else {
result = jfilename.substring(0, 2) + '.';
}
} else {
//TODO deal with UNC names
int index = name.lastIndexOf('/');
normalizedPath = stripTrailingSlashes(normalizedPath, hasDriveLetter ? 3 : 1); // '/foo[/\\]+' -> '/foo/'

if (index == -1) {
if (startsWithDriveLetterOnWindows) {
return jfilename.substring(0, 2) + '.';
} else {
return ".";
}
}
if (index == 0) {
return "/";
}
// stripped off one or more / from trailing slash normalization above...cope with it (C: handled earlier)
if (hasDriveLetter && normalizedPath.length() == 2) return path.substring(0, 3); // C:[/\\]+

if (startsWithDriveLetterOnWindows && index == 2) {
// Include additional path separator
// (so that dirname of "C:\file.txt" is "C:\", not "C:")
index++;
}
int lastSlash = normalizedPath.lastIndexOf('/');
if (lastSlash == -1) return hasDriveLetter ? path.substring(0, 2) + '.' : "."; // 'C:foo' or 'foo' (no slash)
if (lastSlash == 0) return path.substring(0, 1); // '/' or '\'
if (lastSlash == 2 && hasDriveLetter) return path.substring(0, 3); // 'C:[/\\]file'

if (jfilename.startsWith("\\\\")) {
index = jfilename.length();
String[] splitted = jfilename.split(Pattern.quote("\\"));
int last = splitted.length-1;
if (splitted[last].contains(".")) {
index = jfilename.lastIndexOf('\\');
}

}

result = jfilename.substring(0, index);

}

char endChar;
// trim trailing slashes
while (result.length() > minPathLength) {
endChar = result.charAt(result.length() - 1);
if (endChar == '/' || endChar == '\\') {
result = result.substring(0, result.length() - 1);
} else {
break;
if (normalizedPath.startsWith("//")) { // UNC path
if (lastSlash == 1) return path.substring(0, normalizedPath.length()); // '//foo'

if (!normalizedPath.substring(2, lastSlash - 2).contains("/")) { // '//foo/bar'
return path.substring(0, normalizedPath.length());
}

// deeper UNC path
}

return result;
// we strip one last time for case '/foo///////bar.txt'
return stripTrailingSlashes(path.substring(0, lastSlash), hasDriveLetter ? 3 : 1);
}

/**
1 change: 0 additions & 1 deletion spec/tags/1.8/ruby/core/file/dirname_tags.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
windows(JRUBY-4523):File.dirname returns all the components of filename except the last one (edge cases on windows)
windows(JRUBY-4523):File.dirname returns the return all the components of filename except the last one (windows unc)
2 changes: 1 addition & 1 deletion spec/tags/1.9/ruby/core/file/dirname_tags.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
windows(JRUBY-4523):File.dirname returns all the components of filename except the last one (edge cases on windows)
windows(JRUBY-4523):File.dirname returns the return all the components of filename except the last one (windows unc)

2 comments on commit 137a315

@kares
Copy link
Member

@kares kares commented on 137a315 Apr 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems 👍 ... merging to master shall tell since ci is less 🔴

@enebo
Copy link
Member Author

@enebo enebo commented on 137a315 Apr 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kares I just committed another commit (a8eef1f) since I missed a guard to only process UNC if on windows.

Please sign in to comment.