expand_path should use the original encoding#3866
expand_path should use the original encoding#3866ahorek wants to merge 2 commits intojruby:masterfrom
Conversation
|
It appears one of the tests in the JRuby suite failed, but I need to confirm if it's an appropriate test. |
|
Oh, silly me...it's the test you added. Testing on MRI now. |
|
@headius should be fixed now |
|
@ahorek Ok that matches a patch I put together here that I think fits a little better. I'll merge your PR to get the test but I'll go with my patch and you can let me know if it fixes your local issues too. |
|
Looks like with either of our patches I get a couple failures in MRI's suite: Here's the code for that test: def test_expand_path_encoding_filesystem
home = ENV["HOME"]
ENV["HOME"] = "#{DRIVE}/UserHome"
path = "~".encode("US-ASCII")
dir = "C:/".encode("IBM437")
fs = Encoding.find("filesystem")
assert_equal fs, File.expand_path(path).encoding
assert_equal fs, File.expand_path(path, dir).encoding
ensure
ENV["HOME"] = home
endAnd here's the patch I made (applies over your PR): diff --git a/core/src/main/java/org/jruby/RubyFile.java b/core/src/main/java/org/jruby/RubyFile.java
index 23689f1..237fd90 100644
--- a/core/src/main/java/org/jruby/RubyFile.java
+++ b/core/src/main/java/org/jruby/RubyFile.java
@@ -785,13 +785,9 @@ public class RubyFile extends RubyIO implements EncodingCapable {
@JRubyMethod(name = "expand_path", required = 1, optional = 1, meta = true)
public static IRubyObject expand_path19(ThreadContext context, IRubyObject recv, IRubyObject[] args) {
- RubyString path = (RubyString) expandPathInternal(context, recv, args, true, false);
- path.force_encoding(context, context.runtime.getEncodingService().convertEncodingToRubyEncoding(get_path(context, args[0]).getEncoding()));
-
- return path;
+ return expandPathInternal(context, recv, args, true, false);
}
-
/**
* ---------------------------------------------------- File::absolute_path
* File.absolute_path(file_name [, dir_string] ) -> abs_file_name
@@ -1560,6 +1556,7 @@ public class RubyFile extends RubyIO implements EncodingCapable {
RubyString origPath = StringSupport.checkEmbeddedNulls(runtime, get_path(context, args[0]));
String relativePath = origPath.getUnicodeValue();
+ Encoding enc = origPath.getEncoding();
// Special /dev/null of windows
if (Platform.IS_WINDOWS && ("NUL:".equalsIgnoreCase(relativePath) || "NUL".equalsIgnoreCase(relativePath))) {
@@ -1613,7 +1610,7 @@ public class RubyFile extends RubyIO implements EncodingCapable {
// this is basically for classpath:/ and uri:classloader:/
relativePath = relativePath.substring(2).replace('\\', '/');
}
- return concatStrings(runtime, preFix, extra, relativePath);
+ return concatStrings(runtime, preFix, extra, relativePath, enc);
}
String[] uriParts = splitURI(relativePath);
@@ -1627,7 +1624,7 @@ public class RubyFile extends RubyIO implements EncodingCapable {
if (uriParts != null) {
//If the path was an absolute classpath path, return it as-is.
if (uriParts[0].equals("classpath:")) {
- return concatStrings(runtime, preFix, relativePath, postFix);
+ return concatStrings(runtime, preFix, relativePath, postFix, enc);
}
relativePath = uriParts[1];
}
@@ -1745,13 +1742,14 @@ public class RubyFile extends RubyIO implements EncodingCapable {
}
}
}
- return concatStrings(runtime, preFix, realPath, postFix);
+ return concatStrings(runtime, preFix, realPath, postFix, enc);
}
- private static RubyString concatStrings(final Ruby runtime, String s1, String s2, String s3) {
+ private static RubyString concatStrings(final Ruby runtime, String s1, String s2, String s3, Encoding enc) {
return RubyString.newString(runtime,
new StringBuilder(s1.length() + s2.length() + s3.length()).
- append(s1).append(s2).append(s3)
+ append(s1).append(s2).append(s3).toString(),
+ enc
);
}
Unfortunately this is where it gets a bit tricky. Looking at MRI's C code for |
|
Oh, line 660 is the first |
|
@headius Tested, thanks for getting this done! |
fixes #3849