Skip to content

Commit bc7036a

Browse files
eneboenebo
authored
Merge pull request #4177 from jellymann/dirname-with-backslash-on-unix
Fix issues with File.dirname
2 parents 9eaa1bd + 5f2cced commit bc7036a

File tree

3 files changed

+90
-24
lines changed

3 files changed

+90
-24
lines changed

‎core/src/main/java/org/jruby/RubyFile.java‎

Lines changed: 71 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,15 @@ public IRubyObject inspect() {
509509
@JRubyMethod(required = 1, optional = 1, meta = true)
510510
public static IRubyObject basename(ThreadContext context, IRubyObject recv, IRubyObject[] args) {
511511
Ruby runtime = context.runtime;
512+
final String separator = runtime.getClass("File").getConstant("SEPARATOR").toString();
513+
final char separatorChar = separator.charAt(0);
514+
String altSeparator = null;
515+
char altSeparatorChar = '\0';
516+
final IRubyObject rbAltSeparator = runtime.getClass("File").getConstant("ALT_SEPARATOR");
517+
if (rbAltSeparator != context.nil) {
518+
altSeparator = rbAltSeparator.toString();
519+
altSeparatorChar = altSeparator.charAt(0);
520+
}
512521

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

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

549-
// Paths which end in "/" or "\\" must be stripped off.
558+
// Paths which end in File::SEPARATOR or File::ALT_SEPARATOR must be stripped off.
550559
int slashCount = 0;
551560
int length = name.length();
552561
for (int i = length - 1; i >= 0; i--) {
553562
char c = name.charAt(i);
554-
if (c != '/' && c != '\\') {
563+
if (c != separatorChar && (altSeparator == null || c != altSeparatorChar)) {
555564
break;
556565
}
557566
slashCount++;
@@ -560,13 +569,12 @@ public static IRubyObject basename(ThreadContext context, IRubyObject recv, IRub
560569
name = name.substring(0, name.length() - slashCount);
561570
}
562571

563-
int index = name.lastIndexOf('/');
564-
if (index == -1) {
565-
// XXX actually only on windows...
566-
index = name.lastIndexOf('\\');
572+
int index = name.lastIndexOf(separatorChar);
573+
if (altSeparator != null) {
574+
index = Math.max(index, name.lastIndexOf(altSeparatorChar));
567575
}
568576

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

@@ -651,32 +659,57 @@ public static IRubyObject dirname(ThreadContext context, IRubyObject recv, IRuby
651659

652660
static Pattern PROTOCOL_PATTERN = Pattern.compile(URI_PREFIX_STRING + ".*");
653661
public static String dirname(ThreadContext context, String jfilename) {
654-
String name = jfilename.replace('\\', '/');
662+
final Ruby runtime = context.runtime;
663+
final String separator = runtime.getClass("File").getConstant("SEPARATOR").toString();
664+
final char separatorChar = separator.charAt(0);
665+
String altSeparator = null;
666+
char altSeparatorChar = '\0';
667+
final IRubyObject rbAltSeparator = runtime.getClass("File").getConstant("ALT_SEPARATOR");
668+
if (rbAltSeparator != context.nil) {
669+
altSeparator = rbAltSeparator.toString();
670+
altSeparatorChar = altSeparator.charAt(0);
671+
}
672+
String name = jfilename;
673+
if (altSeparator != null) {
674+
name = jfilename.replace(altSeparator, separator);
675+
}
655676
int minPathLength = 1;
656677
boolean trimmedSlashes = false;
657678

679+
boolean startsWithSeparator = false;
680+
681+
if (!name.isEmpty()) {
682+
startsWithSeparator = name.charAt(0) == separatorChar;
683+
}
684+
685+
boolean startsWithUNCOnWindows = Platform.IS_WINDOWS && startsWith(name, separatorChar, separatorChar);
686+
687+
if (startsWithUNCOnWindows) {
688+
minPathLength = 2;
689+
}
690+
658691
boolean startsWithDriveLetterOnWindows = startsWithDriveLetterOnWindows(name);
659692

660693
if (startsWithDriveLetterOnWindows) {
661694
minPathLength = 3;
662695
}
663696

664697
// jar like paths
665-
if (name.contains(".jar!/")) {
666-
int start = name.indexOf("!/") + 1;
698+
if (name.contains(".jar!" + separator)) {
699+
int start = name.indexOf("!" + separator) + 1;
667700
String path = dirname(context, name.substring(start));
668-
if (path.equals(".") || path.equals("/")) path = "";
701+
if (path.equals(".") || path.equals(separator)) path = "";
669702
return name.substring(0, start) + path;
670703
}
671704
// address all the url like paths first
672705
if (PROTOCOL_PATTERN.matcher(name).matches()) {
673-
int start = name.indexOf(":/") + 2;
706+
int start = name.indexOf(":" + separator) + 2;
674707
String path = dirname(context, name.substring(start));
675708
if (path.equals(".")) path = "";
676709
return name.substring(0, start) + path;
677710
}
678711

679-
while (name.length() > minPathLength && name.charAt(name.length() - 1) == '/') {
712+
while (name.length() > minPathLength && name.charAt(name.length() - 1) == separatorChar) {
680713
trimmedSlashes = true;
681714
name = name.substring(0, name.length() - 1);
682715
}
@@ -691,7 +724,7 @@ public static String dirname(ThreadContext context, String jfilename) {
691724
}
692725
} else {
693726
//TODO deal with UNC names
694-
int index = name.lastIndexOf('/');
727+
int index = name.lastIndexOf(separator);
695728

696729
if (index == -1) {
697730
if (startsWithDriveLetterOnWindows) {
@@ -701,7 +734,7 @@ public static String dirname(ThreadContext context, String jfilename) {
701734
}
702735
}
703736
if (index == 0) {
704-
return "/";
737+
return jfilename.substring(0, 1);
705738
}
706739

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

713-
if (startsWith(jfilename, '\\', '\\')) {
714-
index = jfilename.length();
715-
String[] split = jfilename.split(Pattern.quote("\\"));
716-
if (split[ split.length - 1 ].indexOf('.') > -1) {
717-
index = jfilename.lastIndexOf('\\');
746+
if (startsWithUNCOnWindows) {
747+
index = jfilename.length();
748+
String[] split = name.split(Pattern.quote(separator));
749+
int pathSectionCount = 0;
750+
for (int i = 0; i < split.length; i++) {
751+
if (!split[i].isEmpty()) {
752+
pathSectionCount += 1;
718753
}
719-
754+
}
755+
if (pathSectionCount > 2) {
756+
index = name.lastIndexOf(separator);
757+
}
720758
}
721-
722759
result = jfilename.substring(0, index);
760+
}
723761

762+
// trim leading slashes
763+
if (startsWithSeparator && result.length() > minPathLength) {
764+
while (
765+
result.length() > minPathLength &&
766+
(result.charAt(minPathLength) == separatorChar ||
767+
(altSeparator != null && result.charAt(minPathLength) == altSeparatorChar))
768+
) {
769+
result = result.substring(1, result.length());
770+
}
724771
}
725772

726773
char endChar;
727774
// trim trailing slashes
728775
while (result.length() > minPathLength) {
729776
endChar = result.charAt(result.length() - 1);
730-
if (endChar == '/' || endChar == '\\') {
777+
if (endChar == separatorChar || (altSeparator != null && endChar == altSeparatorChar)) {
731778
result = result.substring(0, result.length() - 1);
732779
} else {
733780
break;

‎spec/ruby/core/file/basename_spec.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,20 @@
8888
File.basename("bar.txt.exe", ".txt.exe").should == "bar"
8989
end
9090

91+
it "takes into consideration the platform path separator(s)" do
92+
platform_is_not :windows do
93+
File.basename("C:\\foo\\bar").should == "C:\\foo\\bar"
94+
File.basename("C:/foo/bar").should == "bar"
95+
File.basename("/foo/bar\\baz").should == "bar\\baz"
96+
end
97+
98+
platform_is :windows do
99+
File.basename("C:\\foo\\bar").should == "bar"
100+
File.basename("C:/foo/bar").should == "bar"
101+
File.basename("/foo/bar\\baz").should == "baz"
102+
end
103+
end
104+
91105
it "raises a TypeError if the arguments are not String types" do
92106
lambda { File.basename(nil) }.should raise_error(TypeError)
93107
lambda { File.basename(1) }.should raise_error(TypeError)

‎spec/ruby/core/file/dirname_spec.rb‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@
5353
it "returns all the components of filename except the last one (edge cases on non-windows)" do
5454
File.dirname('/////').should == '/'
5555
File.dirname("//foo//").should == "/"
56+
File.dirname('foo\bar').should == '.'
57+
File.dirname('/foo\bar').should == '/'
58+
File.dirname('foo/bar\baz').should == 'foo'
5659
end
5760
end
5861

@@ -90,6 +93,8 @@
9093
File.dirname("\\\\foo\\bar\\baz").should == "\\\\foo\\bar"
9194
File.dirname("\\\\foo").should =="\\\\foo"
9295
File.dirname("\\\\foo\\bar").should =="\\\\foo\\bar"
96+
File.dirname("\\\\\\foo\\bar").should =="\\\\foo\\bar"
97+
File.dirname("\\\\\\foo").should =="\\\\foo"
9398
end
9499

95100
it "returns the return all the components of filename except the last one (forward_slash)" do

0 commit comments

Comments
 (0)