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.

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

Can't use require_relative with a path beginning with lib in a jar...on Linux #4145

Closed
presidentbeef opened this issue Sep 11, 2016 · 22 comments · Fixed by #4543
Closed

Can't use require_relative with a path beginning with lib in a jar...on Linux #4145

presidentbeef opened this issue Sep 11, 2016 · 22 comments · Fixed by #4543

Comments

@presidentbeef
Copy link

Environment

Provide at least:

  • JRuby version:

jruby 9.1.5.0 (2.3.1) 2016-09-07 036ce39 Java HotSpot(TM) 64-Bit Server VM 25.65-b01 on 1.8.0_65-b17 +jit [linux-x86_64]

  • Operating system and platform

Linux bigblue 4.4.16-desktop-1.mga5 #1 SMP Tue Jul 26 09:23:40 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux

Other relevant info you may wish to add:

  • Installed or activated gems

Just bundler

  • Application/framework version (e.g. Rails, Sinatra)

jrubyfx

Expected Behavior

Load any file in a jar using require_relative

The jrubyfx-9k-jar-problems repo can be re-purposed for this bug, as a follow up to #4000

Actual Behavior

The actual behavior is that require_relative "lib/brakeman_pro" can't find the file. But if you just move the file to lib2/brakeman_pro.rb and require that, it works!

I build the jar like this:

$ gem install bundler
$ TMPDIR=~/tmp/ rake

Then I run

$ java -jar BrakemanPro.jar 
uri:classloader:/vendor/bundle/jruby/2.3.0/gems/jrubyfx-fxmlloader-0.4.1-java/lib/fxmlloader/elts.rb:158: warning: `<<' after local variable or literal is interpreted as binary operator
uri:classloader:/vendor/bundle/jruby/2.3.0/gems/jrubyfx-fxmlloader-0.4.1-java/lib/fxmlloader/elts.rb:158: warning: even though it seems like here document
Errno::ENOENT: No such file or directory - uri:classloader:/usr/lib/brakeman_pro.rb
          realpath at org/jruby/RubyFile.java:812
  require_relative at uri:classloader:/jruby/kernel/kernel.rb:11
            <main> at uri:classloader:/lib/brakeman_pro.rb:4
           require at org/jruby/RubyKernel.java:956
            (root) at uri:classloader:/jruby/kernel/kernel.rb:1
            <main> at classpath:/jar-bootstrap.rb:2

I have tried with JRuby 9.1.3.0, 9.1.4.0, and 9.1.5.0. They all show the same behavior.
A jar built on OSX works on OSX but shows the same error if I run that jar on Linux.

After spending several hours on this, I realized the first require_relative in the bootstrap file works just fine, which baffled me. So I tested requiring other files in different directories and the root directory. They worked just fine. An unsettling realization came over me. I changed move the file to lib2, adjusted the require_relative call and...it worked. 😱

@mkristian
Copy link
Member

@presidentbeef can not reproduce on OSX with jruby-complete-9.1.5.0 using the repository you mentioned. I see

$ java -jar BrakemanPro.jar
uri:classloader:/vendor/bundle/jruby/2.3.0/gems/jrubyfx-fxmlloader-0.4.1-java/lib/fxmlloader/elts.rb:158: warning: `<<' after local variable or literal is interpreted as binary operator
uri:classloader:/vendor/bundle/jruby/2.3.0/gems/jrubyfx-fxmlloader-0.4.1-java/lib/fxmlloader/elts.rb:158: warning: even though it seems like here document
org.eclipse.jface.text.DefaultDocumentAdapter@d24d86c
Sep 12, 2016 8:39:40 AM com.sun.javafx.css.StyleManager loadStylesheetUnPrivileged
INFO: Could not find stylesheet: file:uri:classloader:/lib/resources/css/editor.css

well the missing stylesheet is definitely using an uri-like path which will not find anything. looks like the StyleManager thinks it is a path on the filesystem.

what is your $ env ? maybe that gives some hints where this /usr/lib/brakeman_pro.rb comes from your are seeing

@presidentbeef
Copy link
Author

@mkristian as noted above, this issue does not occur on OS X.

@mkristian
Copy link
Member

@presidentbeef oh -sorry. let me try it on my linux VM then

@presidentbeef
Copy link
Author

@mkristian I think you are right - this is something to do with my environment. I tried it on a completely different Linux machine and it loaded just fine. However - why would require_relative be affected by anything in the environment? It should be strictly checking a known path relative to the current file.

In any case, here is my crufty env:

LC_PAPER=en_US.UTF-8
LC_ADDRESS=en_US.UTF-8
rvm_bin_path=/home/justin/.rvm/bin
LC_MONETARY=en_US.UTF-8
HOSTNAME=bigblue
GEM_HOME=/home/justin/.rvm/gems/jruby-9.1.5.0@test
ANDROID_HOME=/home/justin/pkgs/android-sdk-linux/
GPG_AGENT_INFO=/tmp/gpg-ZS1tDL/S.gpg-agent:4194:1
TERM=xterm
SHELL=/bin/bash
QT5DOCDIR=/usr/share/doc/qt5
CANBERRA_DRIVER=pulse
LC_SOURCED=1
HISTSIZE=1000
IRBRC=/home/justin/.rvm/rubies/jruby-9.1.5.0/.irbrc
QT_XFT=0
TMPDIR=/tmp
MGA_MENU_STYLE=mageia
LC_NUMERIC=en_US.UTF-8
QTDIR=/usr/lib64/qt4
OLDPWD=/home/justin
MY_RUBY_HOME=/home/justin/.rvm/rubies/jruby-9.1.5.0
QTINC=/usr/lib/qt3/include
SSH_TTY=/dev/pts/5
QTDIR5=/usr/lib64/qt5
USER=justin
QT4DOCDIR=/usr/share/doc/qt4
LC_TELEPHONE=en_US.UTF-8
_system_type=Linux
rvm_path=/home/justin/.rvm
SSH_AUTH_SOCK=/tmp/ssh-AMOFAZBvwatn/agent.4168
SYSTEMD_PAGER=/usr/bin/less -FR
SCREENDIR=/home/justin/tmp
rvm_prefix=/home/justin
NLSPATH=/usr/share/locale/%l/%N
MAIL=/var/spool/mail/justin
PATH=/home/justin/.rvm/gems/jruby-9.1.5.0@test/bin:/home/justin/.rvm/gems/jruby-9.1.5.0@global/bin:/home/justin/.rvm/rubies/jruby-9.1.5.0/bin:/home/justin/.rvm/bin:/usr/local/bin:/usr/bin:/usr/local/games:/usr/games:/usr/lib64/qt4/bin:/usr/lib64/qt5/bin:/home/justin/bin:/usr/java/default/bin:/home/justin/work/brat:/home/justin/pkgs/scala-2.10.1/bin:/home/justin/pkgs/android-sdk-linux/platform-tools:/home/justin/pkgs/android-sdk-linux/tools:/home/justin/pkgs/android-sdk-linux/build-tools/17.0.0:/home/justin/pkgs/apache-ant-1.9.6/bin/
LC_MESSAGES=en_US.UTF-8
LC_IDENTIFICATION=en_US.UTF-8
LC_COLLATE=en_US.UTF-8
INPUTRC=/etc/inputrc
PWD=/home/justin/scratch/jrubyfx-9k-jar-problems
EDITOR=vim
TCLLIBPATH=/home/justin/.ActiveTcl/lib/ /home/justin/.ActiveTcl/lib/teapot/package/tcl/lib /home/justin/.ActiveTcl/lib/teapot/package/linux-glibc2.3-x86_64/lib/
LANG=en_US.UTF-8
PYTHONSTARTUP=/etc/pythonrc.py
LC_MEASUREMENT=en_US.UTF-8
_system_arch=x86_64
PS1=\[\][\u@\h \W$(type __git_ps1 >/dev/null  2>&1 && __git_ps1 " (%s)")]\$ \[\]
_system_version=5
SSH_ASKPASS=/usr/lib64/ssh/ssh-askpass
HISTCONTROL=ignoredups
rvm_version=1.27.0 (master)
LESSCHARSET=utf-8
SHLVL=1
HOME=/home/justin
LANGUAGE=en_US.UTF-8:en_US:en
rvm_ruby_string=jruby-9.1.5.0
GCONF_TMPDIR=/tmp
G_FILENAME_ENCODING=@locale
TMP=/tmp
LOGNAME=justin
QTLIB=/usr/lib64
GEM_PATH=/home/justin/.rvm/gems/jruby-9.1.5.0@test:/home/justin/.rvm/gems/jruby-9.1.5.0@global
CLASSPATH=:/home/justin/work/comparison/jars/lights.jar:/home/justin/work/comparison/jars/mina-core-1.1.7.jar:/home/justin/work/comparison/jars/slf4j-api-1.5.5.jar:/home/justin/work/comparison/jars/slf4j-jdk14-1.5.5.jar:/home/justin/work/comparison/
LC_CTYPE=en_US.UTF-8
SSH_CONNECTION=209.49.66.106 64971 192.168.1.69 22
LESSOPEN=|/usr/bin/lesspipe.sh %s
rvm_delete_flag=0
META_CLASS=desktop
RUBY_VERSION=jruby-9.1.5.0
LC_TIME=en_US.UTF-8
_system_name=Mageia
LC_NAME=en_US.UTF-8
_=/usr/bin/env

@mkristian
Copy link
Member

@presidentbeef as I can not reproduce it and the env did not show anything obvious, I just can point you to the require_relative method: https://github.com/jruby/jruby/blob/master/core/src/main/ruby/jruby/kernel/kernel.rb#L3

there is some conversion going on like File.realpath and File.dirname and File.expand_path - plenty of places where those uri-like paths can go wrong :(

@presidentbeef
Copy link
Author

Thanks - in some ways it is a relief to know this isn't a general problem :)

@headius
Copy link
Member

headius commented Sep 13, 2016

@presidentbeef Perhaps on that one Linux environment you have a lib in your current directory, and on the other platforms you don't?

Perhaps you can play around with our require_relative implementation (copied out and named something else) in your bad environment, tweaking and putsing to get more information? Without a way to reproduce this we're stuck guessing :-\

@presidentbeef
Copy link
Author

I am continuing to look into this because things are getting weird.

First of all, now I can't reproduce it with the jrubyfx-9k-jar-problems repo either. Which is pretty strange.

Secondly, I thought it was require_relative "lib/brakeman_pro" that was failing. But it's not - it's a require_relative inside lib/brakeman_pro.rb for a different file.

Thirdly, I didn't realize this isn't a LoadError which would be raised if the file couldn't be found. It's throwing an Errno::ENOENT exception from https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/RubyFile.java#L808-L814 So it seems to me it's having trouble expanding the path correctly, as suggested above.

Sorry for the confusion, I've been fighting this for a few days and my brain hurts a little.

@presidentbeef
Copy link
Author

I have a much simpler repro repo: https://github.com/presidentbeef/jruby-realpath-error

It comes down to behavior of File.realpath.

$ cp ~/.jruby-jar/jruby-complete-9.1.5.0.jar myapp.jar
$ jar ufe myapp.jar org.jruby.JarBootstrapMain jar-bootstrap.rb lib/test.rb
$ java -jar myapp.jar 
Errno::ENOENT: No such file or directory - uri:classloader:/usr/lib/test.rb
  realpath at org/jruby/RubyFile.java:812
    <main> at uri:classloader:/lib/test.rb:1
   require at org/jruby/RubyKernel.java:956
    (root) at uri:classloader:/jruby/kernel/kernel.rb:1
    <main> at classpath:/jar-bootstrap.rb:1

However, this only happens on one machine. The error goes away if I rename lib/ to lib2/. It goes away if test.rb is in the root directory instead of lib/.

Still scratching my head about it.

@headius
Copy link
Member

headius commented Sep 30, 2016

I imagine the renaming changes behavior because somewhere along the line it doesn't see a real system path anymore, so it goes back to searching the classloader.

Thanks for the reproduction! That will definitely help us sort out what's happening here.

@headius
Copy link
Member

headius commented Nov 8, 2016

@mkristian I don't suppose you have a fix up your sleeve for 9.1.6.0? Otherwise we'll have to punt.

@headius headius modified the milestones: JRuby 9.1.7.0, JRuby 9.1.6.0 Nov 8, 2016
@headius
Copy link
Member

headius commented Nov 8, 2016

Too late!

@enebo enebo modified the milestones: JRuby 9.1.8.0, JRuby 9.1.7.0 Jan 10, 2017
@headius headius modified the milestones: JRuby 9.2.0.0, JRuby 9.1.8.0 Mar 3, 2017
@headius
Copy link
Member

headius commented Mar 3, 2017

I'm still unclear what's happening here. Do you have any updates @presidentbeef? I've retargeted for 9.2 but this is in danger of slipping forever.

@presidentbeef
Copy link
Author

Considering the repro has been condensed down to File.realpath(__FILE__), I am fairly certain the issue lies somewhere in expandPathInternal.

I will do my best to take a look and see if I can figure out where/why things are going wrong.

@presidentbeef
Copy link
Author

I have located what I believe to be the smoking gun.

In my example (and in the real app), the __FILE__ of the second file loaded (in this case lib/test.rb) is "uri:classloader:/lib/test.rb". This is consistent across platforms.

If I am tracing the code right, this is the code that is execuded in expandPathInternal.

Note the call to canonicalizePath(relativePath.substring(offset)). This code grabs the bit after uri:classloader: and calls Java's .getCanonicalPath on it.

NOW IT GETS INTERESTING

On Mageia Linux:

jruby-9.1.7.0 :001 > java.io.File.new("/lib/test.rb").getCanonicalPath()
 => "/usr/lib/test.rb" 
jruby-9.1.7.0 :002 > java.io.File.new("/lib2/test.rb").getCanonicalPath()
 => "/lib2/test.rb" 

On Mac OSX and Ubuntu Linux:

jruby-9.1.7.0 :001 > java.io.File.new("/lib/test.rb").getCanonicalPath()
 => "/lib/test.rb" 
jruby-9.1.7.0 :002 > java.io.File.new("/lib2/test.rb").getCanonicalPath()
 => "/lib2/test.rb" 

The version of Java does not seem to matter (same results with OpenJDK and Oracle Java SE).

On Mageia I have:

openjdk version "1.8.0_121"
OpenJDK Runtime Environment (build 1.8.0_121-b14)
OpenJDK 64-Bit Server VM (build 25.121-b14, mixed mode)

On Ubuntu:

openjdk version "1.8.0_121"
OpenJDK Runtime Environment (build 1.8.0_121-8u121-b13-0ubuntu1.16.04.2-b13)
OpenJDK 64-Bit Server VM (build 25.121-b13, mixed mode)

I can reproduce the issue in pure Java. So I don't think this is a JRuby problem at all.

@ptoomey3
Copy link
Contributor

I played around with this for a minute and think I have it. It looks like java seems to resolve symbolic links when doing file.getCanonicalPath(). On Mageia, /lib is a symbolic link to /usr/lib(not the case on Ubuntu). You can get the same behavior on OS X by doing something like doing a file.getCanonicalPath() with a path in /tmp, since /tmp is a symbolic link to /private/tmp on OS X.

import java.io.File;

class Test {
  public static void main(String[] args) throws Exception {
    File foo = new File("/tmp/foo.rb");
    System.out.println(foo.getCanonicalPath());
  }
}

Patricks-MacBook-Pro:Downloads ptoomey3$ java Test
/private/tmp/foo.rb

@mkristian
Copy link
Member

wow - great findings. so getCanonicalPath() is not a good idea for uniform the path on something like uri:classloader:/lib/test.rb

@presidentbeef
Copy link
Author

Holy cow @ptoomey3! I thought this was a lost cause 😀 Amazing work.

@headius
Copy link
Member

headius commented Mar 23, 2017

@ptoomey3 Yeah great investigation!

So I think we just need to stop calling path canonicalization methods on all in-jar or in-classpath paths, eh?

@headius
Copy link
Member

headius commented Mar 23, 2017

I am testing the following patch:

diff --git a/core/src/main/java/org/jruby/RubyFile.java b/core/src/main/java/org/jruby/RubyFile.java
index 9a861ab..11c0057 100644
--- a/core/src/main/java/org/jruby/RubyFile.java
+++ b/core/src/main/java/org/jruby/RubyFile.java
@@ -1638,32 +1638,39 @@ public class RubyFile extends RubyIO implements EncodingCapable {
             int index = relativePath.indexOf("!/");
             postFix = relativePath.substring(index);
             relativePath = relativePath.substring(0, index);
-        }
-        else if (protocol.find()) {
+        } else if (protocol.find()) {
             preFix = protocol.group();
             int offset = protocol.end();
             String extra = "";
             int index = relativePath.indexOf("file://");
+            boolean classloaderURI = preFix.equals("uri:classloader:") || preFix.equals("classpath:");
+
             if (index >= 0) {
                 index += 7; // "file://".length == 7
                 // chck if its "file:///"
                 if (relativePath.length() > index && relativePath.charAt(index) == '/') {
                     offset += 2; extra = "//";
-                }
-                else {
+                } else {
                     offset += 1; extra = "/";
                 }
+            } else {
+                if (classloaderURI && relativePath.startsWith("//", offset)) {
+                    // on Windows "uri:classloader://home" ends up as "//home" - trying a network drive!
+                    offset += 1; // skip one '/'
+                }
             }
-            else if ( ( preFix.equals("uri:classloader:") || preFix.equals("classpath:") )
-                    && relativePath.startsWith("//", offset) ) {
-                // on Windows "uri:classloader://home" ends up as "//home" - trying a network drive!
-                offset += 1; // skip one '/'
+
+            relativePath = relativePath.substring(offset);
+
+            if (!classloaderURI) {
+                relativePath = canonicalizePath(relativePath);
             }
-            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 concatStrings(runtime, preFix, extra, relativePath, enc);
         }
 

This skips the canonicalization for any classloader URIs:

~/projects/jruby $ rvm jruby-9.1.8.0 do jruby -v -e "p File.absolute_path('uri:classloader:/lib/blah')"
jruby 9.1.8.0 (2.3.1) 2017-03-06 90fc7ab OpenJDK 64-Bit Server VM 25.121-b14 on 1.8.0_121-b14 +jit [linux-x86_64]
"uri:classloader:/usr/lib/blah"

~/projects/jruby $ jruby -v -e "p File.absolute_path('uri:classloader:/lib/blah')"
jruby 9.1.9.0-SNAPSHOT (2.3.3) 2017-03-16 7ce6c89 OpenJDK 64-Bit Server VM 25.121-b14 on 1.8.0_121-b14 +jit [linux-x86_64]
"uri:classloader:/lib/blah"

@headius
Copy link
Member

headius commented Mar 23, 2017

Ok, I realized with my patch that we do expect canonicalization of classpath URIs to expand filesystem relative paths like "/lib/../foo". I have modified my patch to canonicalize classpath URIs using a bogus path prefix that will never resolve to anything on the actual filesystem. Seems to be working ok.

headius added a commit to headius/jruby that referenced this issue Mar 23, 2017
This prevents in-classloader paths from expanding using real
filesystem paths that may resolve symlinks.

Fixes jruby#4145.
@headius
Copy link
Member

headius commented Mar 23, 2017

Pushed a PR, we'll see how it feels.

@headius headius modified the milestones: JRuby 9.1.9.0, JRuby 9.2.0.0 Mar 23, 2017
kares added a commit to kares/jruby that referenced this issue Jun 5, 2017
has been attempted at cbf9a7d
... using a fake root path, however on Windows /SMT expands as C:\SMT

added '__' at the end so its more obvious if this is slightly off again

closes jruby#4630 (also jruby#4645)
headius added a commit that referenced this issue Jun 6, 2017
help classpath: canonicalization on Windows to fully resolve #4145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants