REGRESSION - File.realpath fails on classpath paths #4612

Closed
trejkaz opened this Issue May 18, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@trejkaz

trejkaz commented May 18, 2017

Environment

JRuby 9.1.9.0
Windows only

Test Case

@Test
public void testResolveClasspath() throws Exception
{
    ScriptEngine engine = createEngine();

    String path = "classpath:/path/to/this/TestRubyFile.class";
    String result = (String) engine.eval("File.realpath('" + path + "')");
    assertThat(result, is(equalTo(path)));
}

protected ScriptEngine createEngine()
{
    return new JRubyEngineFactory().getScriptEngine();
}

Expected Behavior

Test should pass, like it did on JRuby 9.0.5.0.

Actual Behavior

Test now fails:

Caused by: org.jruby.exceptions.RaiseException: (Errno::ENOENT) classpath:BY\path\to\this\TestRubyFile.class
at org.jruby.RubyFile.realpath(org/jruby/RubyFile.java:864)
at RUBY.<main>(<script>:1)

I'm not sure where it pulled the "BY" from, but the slashes are in the wrong direction too.

@trejkaz

This comment has been minimized.

Show comment
Hide comment
@trejkaz

trejkaz May 18, 2017

Problem appears to be here:

        else if (protocol.find()) {
            preFix = protocol.group();
            int offset = protocol.end();
            String extra = "";
            if (relativePath.contains("file://")) {
                if (relativePath.contains("file:///")) {
                    offset += 2;
                    extra = "//";
                }
                else {
                    offset += 1;
                    extra = "/";
                }
            }
            relativePath = canonicalizePath(relativePath.substring(offset));
            if (Platform.IS_WINDOWS && !preFix.contains("file:") && startsWithDriveLetterOnWindows(relativePath)) {
                // this is basically for classpath:/ and uri:classloader:/
                relativePath = relativePath.substring(2).replace("\\", "/");
            }
            return runtime.newString(preFix + extra + relativePath);
        }

This call to canonicalizePath is implemented as follows:

    private static String canonicalizePath(String path) {
        try {
            return new File(path).getCanonicalPath();
        } catch (IOException ignore) {
            return path;
        }
    }

That File#getCanonicalPath() call will return the path with backslashes, so now it's already broken. Still not quite sure how a "BY\" ends up in there as well.

trejkaz commented May 18, 2017

Problem appears to be here:

        else if (protocol.find()) {
            preFix = protocol.group();
            int offset = protocol.end();
            String extra = "";
            if (relativePath.contains("file://")) {
                if (relativePath.contains("file:///")) {
                    offset += 2;
                    extra = "//";
                }
                else {
                    offset += 1;
                    extra = "/";
                }
            }
            relativePath = canonicalizePath(relativePath.substring(offset));
            if (Platform.IS_WINDOWS && !preFix.contains("file:") && startsWithDriveLetterOnWindows(relativePath)) {
                // this is basically for classpath:/ and uri:classloader:/
                relativePath = relativePath.substring(2).replace("\\", "/");
            }
            return runtime.newString(preFix + extra + relativePath);
        }

This call to canonicalizePath is implemented as follows:

    private static String canonicalizePath(String path) {
        try {
            return new File(path).getCanonicalPath();
        } catch (IOException ignore) {
            return path;
        }
    }

That File#getCanonicalPath() call will return the path with backslashes, so now it's already broken. Still not quite sure how a "BY\" ends up in there as well.

@kares kares added the regression label May 23, 2017

@trejkaz

This comment has been minimized.

Show comment
Hide comment
@trejkaz

trejkaz May 30, 2017

Seems to have been fixed in 9.1.10.0.

trejkaz commented May 30, 2017

Seems to have been fixed in 9.1.10.0.

@trejkaz

This comment has been minimized.

Show comment
Hide comment
@trejkaz

trejkaz Jun 22, 2017

The new behaviour is:

Expected: is "classpath:/org/joda/time/DateTime.class"
     but: was "classpath:\org\joda\time\DateTime.class"

I remember I had commented on slash direction in File in the past, and was told that because MRI always produced forward slashes, JRuby also produced forward slashes... but is this not the case anymore? Would be fine by me. Although... I don't know about producing backslashes in a classpath... maybe it's OK?

trejkaz commented Jun 22, 2017

The new behaviour is:

Expected: is "classpath:/org/joda/time/DateTime.class"
     but: was "classpath:\org\joda\time\DateTime.class"

I remember I had commented on slash direction in File in the past, and was told that because MRI always produced forward slashes, JRuby also produced forward slashes... but is this not the case anymore? Would be fine by me. Although... I don't know about producing backslashes in a classpath... maybe it's OK?

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Jun 23, 2017

Member
Member

mkristian commented Jun 23, 2017

@trejkaz

This comment has been minimized.

Show comment
Hide comment
@trejkaz

trejkaz Jun 26, 2017

Tried uri:classloader:/... - it fails with AccessControlException when it tries to read IDEA's jar here:

at java.util.jar.JarFile.<init>(JarFile.java:103)
at org.jruby.util.JarCache$JarIndex.<init>(JarCache.java:46)

trejkaz commented Jun 26, 2017

Tried uri:classloader:/... - it fails with AccessControlException when it tries to read IDEA's jar here:

at java.util.jar.JarFile.<init>(JarFile.java:103)
at org.jruby.util.JarCache$JarIndex.<init>(JarCache.java:46)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment