Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix issues with File.dirname #4177

Merged
merged 6 commits into from Nov 6, 2016
Merged

Fix issues with File.dirname #4177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aef4bad
fix File.dirname with backslashes in unix
Sep 3, 2016
db355da
Merge remote-tracking branch 'upstream/master' into dirname-with-back…
Sep 24, 2016
f20829c
Fix File.dirname on Windows (including UNC)
Sep 24, 2016
3eb2550
Remove jruby deviation from File.split spec
Oct 8, 2016
580a2e6
use SEPARATOR and ALT_SEPARATOR in File.basename
Oct 23, 2016
5f2cced
Fix basename with mixed slashes on windows
Oct 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
95 changes: 71 additions & 24 deletions core/src/main/java/org/jruby/RubyFile.java
View file
Open in desktop
Expand Up @@ -509,6 +509,15 @@ public IRubyObject inspect() {
@JRubyMethod(required = 1, optional = 1, meta = true)
public static IRubyObject basename(ThreadContext context, IRubyObject recv, IRubyObject[] args) {
Ruby runtime = context.runtime;
final String separator = runtime.getClass("File").getConstant("SEPARATOR").toString();
final char separatorChar = separator.charAt(0);
String altSeparator = null;
char altSeparatorChar = '\0';
final IRubyObject rbAltSeparator = runtime.getClass("File").getConstant("ALT_SEPARATOR");
if (rbAltSeparator != context.nil) {
altSeparator = rbAltSeparator.toString();
altSeparatorChar = altSeparator.charAt(0);
}

RubyString origString = StringSupport.checkEmbeddedNulls(runtime, get_path(context, args[0]));
Encoding origEncoding = origString.getEncoding();
Expand Down Expand Up @@ -542,16 +551,16 @@ public static IRubyObject basename(ThreadContext context, IRubyObject recv, IRub
}
}

while (name.length() > 1 && name.charAt(name.length() - 1) == '/') {
while (name.length() > 1 && (name.charAt(name.length() - 1) == separatorChar || (altSeparator != null && name.charAt(name.length() - 1) == altSeparatorChar))) {
name = name.substring(0, name.length() - 1);
}

// Paths which end in "/" or "\\" must be stripped off.
// Paths which end in File::SEPARATOR or File::ALT_SEPARATOR must be stripped off.
int slashCount = 0;
int length = name.length();
for (int i = length - 1; i >= 0; i--) {
char c = name.charAt(i);
if (c != '/' && c != '\\') {
if (c != separatorChar && (altSeparator == null || c != altSeparatorChar)) {
break;
}
slashCount++;
Expand All @@ -560,13 +569,12 @@ public static IRubyObject basename(ThreadContext context, IRubyObject recv, IRub
name = name.substring(0, name.length() - slashCount);
}

int index = name.lastIndexOf('/');
if (index == -1) {
// XXX actually only on windows...
index = name.lastIndexOf('\\');
int index = name.lastIndexOf(separatorChar);
if (altSeparator != null) {
index = Math.max(index, name.lastIndexOf(altSeparatorChar));
}

if (!name.equals("/") && index != -1) {
if (!(name.equals(separator) || (altSeparator != null && name.equals(altSeparator))) && index != -1) {
name = name.substring(index + 1);
}

Expand Down Expand Up @@ -651,32 +659,57 @@ 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('\\', '/');
final Ruby runtime = context.runtime;
final String separator = runtime.getClass("File").getConstant("SEPARATOR").toString();
final char separatorChar = separator.charAt(0);
String altSeparator = null;
char altSeparatorChar = '\0';
final IRubyObject rbAltSeparator = runtime.getClass("File").getConstant("ALT_SEPARATOR");
if (rbAltSeparator != context.nil) {
altSeparator = rbAltSeparator.toString();
altSeparatorChar = altSeparator.charAt(0);
}
String name = jfilename;
if (altSeparator != null) {
name = jfilename.replace(altSeparator, separator);
}
int minPathLength = 1;
boolean trimmedSlashes = false;

boolean startsWithSeparator = false;

if (!name.isEmpty()) {
startsWithSeparator = name.charAt(0) == separatorChar;
}

boolean startsWithUNCOnWindows = Platform.IS_WINDOWS && startsWith(name, separatorChar, separatorChar);

if (startsWithUNCOnWindows) {
minPathLength = 2;
}

boolean startsWithDriveLetterOnWindows = startsWithDriveLetterOnWindows(name);

if (startsWithDriveLetterOnWindows) {
minPathLength = 3;
}

// jar like paths
if (name.contains(".jar!/")) {
int start = name.indexOf("!/") + 1;
if (name.contains(".jar!" + separator)) {
int start = name.indexOf("!" + separator) + 1;
String path = dirname(context, name.substring(start));
if (path.equals(".") || path.equals("/")) path = "";
if (path.equals(".") || path.equals(separator)) 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;
int start = name.indexOf(":" + separator) + 2;
String path = dirname(context, name.substring(start));
if (path.equals(".")) path = "";
return name.substring(0, start) + path;
}

while (name.length() > minPathLength && name.charAt(name.length() - 1) == '/') {
while (name.length() > minPathLength && name.charAt(name.length() - 1) == separatorChar) {
trimmedSlashes = true;
name = name.substring(0, name.length() - 1);
}
Expand All @@ -691,7 +724,7 @@ public static String dirname(ThreadContext context, String jfilename) {
}
} else {
//TODO deal with UNC names
int index = name.lastIndexOf('/');
int index = name.lastIndexOf(separator);

if (index == -1) {
if (startsWithDriveLetterOnWindows) {
Expand All @@ -701,7 +734,7 @@ public static String dirname(ThreadContext context, String jfilename) {
}
}
if (index == 0) {
return "/";
return jfilename.substring(0, 1);
}

if (startsWithDriveLetterOnWindows && index == 2) {
Expand All @@ -710,24 +743,38 @@ public static String dirname(ThreadContext context, String jfilename) {
index++;
}

if (startsWith(jfilename, '\\', '\\')) {
index = jfilename.length();
String[] split = jfilename.split(Pattern.quote("\\"));
if (split[ split.length - 1 ].indexOf('.') > -1) {
index = jfilename.lastIndexOf('\\');
if (startsWithUNCOnWindows) {
index = jfilename.length();
String[] split = name.split(Pattern.quote(separator));
int pathSectionCount = 0;
for (int i = 0; i < split.length; i++) {
if (!split[i].isEmpty()) {
pathSectionCount += 1;
}

}
if (pathSectionCount > 2) {
index = name.lastIndexOf(separator);
}
}

result = jfilename.substring(0, index);
}

// trim leading slashes
if (startsWithSeparator && result.length() > minPathLength) {
while (
result.length() > minPathLength &&
(result.charAt(minPathLength) == separatorChar ||
(altSeparator != null && result.charAt(minPathLength) == altSeparatorChar))
) {
result = result.substring(1, result.length());
}
}

char endChar;
// trim trailing slashes
while (result.length() > minPathLength) {
endChar = result.charAt(result.length() - 1);
if (endChar == '/' || endChar == '\\') {
if (endChar == separatorChar || (altSeparator != null && endChar == altSeparatorChar)) {
result = result.substring(0, result.length() - 1);
} else {
break;
Expand Down
14 changes: 14 additions & 0 deletions spec/ruby/core/file/basename_spec.rb
View file
Open in desktop
Expand Up @@ -91,6 +91,20 @@
end
end

it "takes into consideration the platform path separator(s)" do
platform_is_not :windows do
File.basename("C:\\foo\\bar").should == "C:\\foo\\bar"
File.basename("C:/foo/bar").should == "bar"
File.basename("/foo/bar\\baz").should == "bar\\baz"
end

platform_is :windows do
File.basename("C:\\foo\\bar").should == "bar"
File.basename("C:/foo/bar").should == "bar"
File.basename("/foo/bar\\baz").should == "baz"
end
end

it "raises a TypeError if the arguments are not String types" do
lambda { File.basename(nil) }.should raise_error(TypeError)
lambda { File.basename(1) }.should raise_error(TypeError)
Expand Down
5 changes: 5 additions & 0 deletions spec/ruby/core/file/dirname_spec.rb
View file
Open in desktop
Expand Up @@ -53,6 +53,9 @@
it "returns all the components of filename except the last one (edge cases on non-windows)" do
File.dirname('/////').should == '/'
File.dirname("//foo//").should == "/"
File.dirname('foo\bar').should == '.'
File.dirname('/foo\bar').should == '/'
File.dirname('foo/bar\baz').should == 'foo'
end
end

Expand Down Expand Up @@ -90,6 +93,8 @@
File.dirname("\\\\foo\\bar\\baz").should == "\\\\foo\\bar"
File.dirname("\\\\foo").should =="\\\\foo"
File.dirname("\\\\foo\\bar").should =="\\\\foo\\bar"
File.dirname("\\\\\\foo\\bar").should =="\\\\foo\\bar"
File.dirname("\\\\\\foo").should =="\\\\foo"
end

it "returns the return all the components of filename except the last one (forward_slash)" do
Expand Down
18 changes: 3 additions & 15 deletions spec/ruby/core/file/split_spec.rb
View file
Open in desktop
Expand Up @@ -27,21 +27,9 @@
end

platform_is_not os: :windows do
not_compliant_on :jruby do
it "does not split a string that contains '\\'" do
File.split(@backslash).should == [".", "C:\\foo\\bar\\baz"]
File.split(@backslash_ext).should == [".", "C:\\foo\\bar\\baz.rb"]
end
end

deviates_on :jruby do
it "splits the string at the last '\\' when the last component does not have an extension" do
File.split(@backslash).should == ["C:\\foo\\bar", "baz"]
end

it "splits the string at the last '\\' when the last component has an extension" do
File.split(@backslash_ext).should == ["C:\\foo\\bar", "baz.rb"]
end
it "does not split a string that contains '\\'" do
File.split(@backslash).should == [".", "C:\\foo\\bar\\baz"]
File.split(@backslash_ext).should == [".", "C:\\foo\\bar\\baz.rb"]
end
end

Expand Down