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

Backtick behavior around symlinks changed from 1.7.19 to 1.7.22 #3459

Closed
hydrogen18 opened this issue Nov 12, 2015 · 13 comments
Closed

Backtick behavior around symlinks changed from 1.7.19 to 1.7.22 #3459

hydrogen18 opened this issue Nov 12, 2015 · 13 comments

Comments

@hydrogen18
Copy link

I'm trying to upgrade to JRuby 1.7.22 from 1.7.19. Somehow the backticks (subshell) behavior changed. It looks like symlinks are handled differently between the versions. This is causing invocation of pg_dump to fail.

Its almost like 1.7.22 is somehow following the symlink itself and invoking the pg_wrapper shell script directly. Is there some configuration option around this?

[ericu-destroyer-of-worlds] jruby$ rvm use jruby-1.7.19
Using /home/ericu/.rvm/gems/jruby-1.7.19
[ericu-destroyer-of-worlds] jruby$ irb
jruby-1.7.19 :001 > `pg_dump`
 => "--\n-- PostgreSQL database dump\n--\n\nSET statement_timeout = 0;\nSET lock_timeout = 0;\nSET client_encoding = 'UTF8';\nSET standard_conforming_strings = on;\nSET check_function_bodies = false;\nSET client_min_messages = warning;\n\n--\n-- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: \n--\n\nCREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;\n\n\n--\n-- Name: EXTENSION plpgsql; Type: COMMENT; Schema: -; Owner: \n--\n\nCOMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';\n\n\nSET search_path = public, pg_catalog;\n\nSET default_tablespace = '';\n\nSET default_with_oids = false;\n\n--\n-- Name: bobs; Type: TABLE; Schema: public; Owner: ericu; Tablespace: \n--\n\nCREATE TABLE bobs (\n    id integer NOT NULL\n);\n\n\nALTER TABLE bobs OWNER TO ericu;\n\n--\n-- Data for Name: bobs; Type: TABLE DATA; Schema: public; Owner: ericu\n--\n\nCOPY bobs (id) FROM stdin;\n\\.\n\n\n--\n-- Name: foobar; Type: CONSTRAINT; Schema: public; Owner: ericu; Tablespace: \n--\n\nALTER TABLE ONLY bobs\n    ADD CONSTRAINT foobar UNIQUE (id);\n\n\n--\n-- Name: public; Type: ACL; Schema: -; Owner: postgres\n--\n\nREVOKE ALL ON SCHEMA public FROM PUBLIC;\nREVOKE ALL ON SCHEMA public FROM postgres;\nGRANT ALL ON SCHEMA public TO postgres;\nGRANT ALL ON SCHEMA public TO PUBLIC;\n\n\n--\n-- PostgreSQL database dump complete\n--\n\n" 
jruby-1.7.19 :002 > 
[ericu-destroyer-of-worlds] jruby$ rvm use jruby-1.7.22
Using /home/ericu/.rvm/gems/jruby-1.7.22
[ericu-destroyer-of-worlds] jruby$ irb
jruby-1.7.22 :001 > `pg_dump`
Error: pg_wrapper should not be called directly, but through a symlink
 => "" 
jruby-1.7.22 :002 > 
[ericu-destroyer-of-worlds] jruby$ ls -l `which pg_dump`
lrwxrwxrwx 1 root root 37 Oct  8 06:53 /usr/bin/pg_dump -> ../share/postgresql-common/pg_wrapper
@hydrogen18
Copy link
Author

Maybe this is the problem?

jruby-1.7.22 :002 > java_import org.jruby.util.ShellLauncher
 => [Java::OrgJrubyUtil::ShellLauncher] 
jruby-1.7.22 :003 > ShellLauncher.should_use_shell
ArgumentError: wrong number of arguments (0 for 1)
    from (irb):3:in `evaluate'
    from org/jruby/RubyKernel.java:1079:in `eval'
    from org/jruby/RubyKernel.java:1479:in `loop'
    from org/jruby/RubyKernel.java:1242:in `catch'
    from org/jruby/RubyKernel.java:1242:in `catch'
    from /home/ericu/.rvm/rubies/jruby-1.7.22/bin/irb:13:in `(root)'
jruby-1.7.22 :004 > ShellLauncher.should_use_shell('pg_dump')
 => false 

Can I override the behavior of ShellLauncher somehow?

@hydrogen18
Copy link
Author

It looks like ShellLauncher.java calls File.getCanonicalPath()

        public void verifyExecutableForDirect() {
            if (isCmdBuiltin(args[0].trim())) {
                execArgs = new String[args.length + 2];
                execArgs[0] = shell;
                execArgs[1] = "/c";
                execArgs[2] = args[0].trim();
                System.arraycopy(args, 1, execArgs, 3, args.length - 1);
            } else {
                verifyExecutable();
                execArgs = args;
                try {
                    execArgs[0] = executableFile.getCanonicalPath();
                } catch (IOException ioe) {
                    // can't get the canonical path, will use as-is
                }
            }
        }

I think this is a problem.

@hydrogen18
Copy link
Author

I copied core/src/main/ruby/jruby/kernel/jruby/process_manager.rb to my home directory and changed it to have use_shell = true. It works after I require it

[ericu-destroyer-of-worlds] ~$ irb
jruby-1.7.22 :001 > `pg_dump`
Error: pg_wrapper should not be called directly, but through a symlink
 => "" 
jruby-1.7.22 :002 > require 'process_manager'
/home/ericu/process_manager.rb:13 warning: already initialized constant Redirect
/home/ericu/process_manager.rb:14 warning: already initialized constant LaunchConfig
/home/ericu/process_manager.rb:15 warning: already initialized constant JFile
 => true 
jruby-1.7.22 :003 > `pg_dump`
 => "--\n-- PostgreSQL database dump\n--\n\nSET statement_timeout = 0;\nSET lock_timeout = 0;\nSET client_encoding = 'UTF8';\nSET standard_conforming_strings = on;\nSET check_function_bodies = false;\nSET client_min_messages = warning;\n\n--\n-- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: \n--\n\nCREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;\n\n\n--\n-- Name: EXTENSION plpgsql; Type: COMMENT; Schema: -; Owner: \n--\n\nCOMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';\n\n\nSET search_path = public, pg_catalog;\n\nSET default_tablespace = '';\n\nSET default_with_oids = false;\n\n--\n-- Name: bobs; Type: TABLE; Schema: public; Owner: ericu; Tablespace: \n--\n\nCREATE TABLE bobs (\n    id integer NOT NULL\n);\n\n\nALTER TABLE bobs OWNER TO ericu;\n\n--\n-- Data for Name: bobs; Type: TABLE DATA; Schema: public; Owner: ericu\n--\n\nCOPY bobs (id) FROM stdin;\n\\.\n\n\n--\n-- Name: foobar; Type: CONSTRAINT; Schema: public; Owner: ericu; Tablespace: \n--\n\nALTER TABLE ONLY bobs\n    ADD CONSTRAINT foobar UNIQUE (id);\n\n\n--\n-- Name: public; Type: ACL; Schema: -; Owner: postgres\n--\n\nREVOKE ALL ON SCHEMA public FROM PUBLIC;\nREVOKE ALL ON SCHEMA public FROM postgres;\nGRANT ALL ON SCHEMA public TO postgres;\nGRANT ALL ON SCHEMA public TO PUBLIC;\n\n\n--\n-- PostgreSQL database dump complete\n--\n\n" 
jruby-1.7.22 :004 > 

@kares kares added this to the JRuby 1.7.25 milestone Mar 4, 2016
kares added a commit to kares/activerecord-jdbc-adapter that referenced this issue Mar 4, 2016
kares added a commit to jruby/activerecord-jdbc-adapter that referenced this issue Mar 7, 2016
…ession on 1.7.19

test with older 1.7.18 until jruby/jruby#3459 gets resolved
@enebo
Copy link
Member

enebo commented Apr 8, 2016

@kares how do I reproduce this? I tried an irb session on MacOS where I did:

ln -s /bin/ls ~/bin/my_ls

Then in irb I ran my_ls and it worked fine.

@kares
Copy link
Member

kares commented Apr 8, 2016

not sure (haven't tried) but it definitely reproduced with pg_dump (postgres being installed on Ubuntu).

@hydrogen18
Copy link
Author

Let us ignore postgres & ubuntu. They have nothing to do with this problem, I was just providing details.

Create a file running_as.c and put in it the following content

#include "stdio.h"

int main(int argc, char* argv[]){ 
  printf("I am being ran as:%s\n", argv[0]);
}

In case you are not familiar with C: This program just displays the value of argv[0]

Compile it with the following command

gcc -o ./running_as ./running_as.c

This will give you an executable called 'running_as. Create a symlink to it calledfoobar`

 ln -s ./running_as ./foobar

Using your favorite shell, invoke it directly and via the symlink

ericu@ericu-desktop:~$ ./running_as 
I am being ran as:./running_as
ericu@ericu-desktop:~$ ./foobar 
I am being ran as:./foobar
ericu@ericu-desktop:~$ 

Notice the output is different. Start irb using jruby 1.7.19. Use the backticks to invoke the processes

jruby-1.7.19 :001 > `./running_as`
 => "I am being ran as:./running_as\n" 
jruby-1.7.19 :002 > `./foobar`
 => "I am being ran as:./foobar\n" 
jruby-1.7.19 :003 > 

Do the same thing with jruby 1.7.22

jruby-1.7.22 :001 > `./running_as`
 => "I am being ran as:/home/ericu/running_as\n" 
jruby-1.7.22 :002 > `./foobar`
 => "I am being ran as:/home/ericu/running_as\n" 
jruby-1.7.22 :003 > 

This is blatantly not the expected behavior. Nowhere in the definition of ruby's backticks does it say "derefence a symlink". Here is ruby 1.9.3 for comparison

1.9.3-p547 :001 > `./running_as`
 => "I am being ran as:./running_as\n" 
1.9.3-p547 :002 > `./foobar`
 => "I am being ran as:./foobar\n" 
1.9.3-p547 :003 > 

The cause of the bug is the usage of the method getCanonicalPath on the java.io.File object. My guess is someone saw "canonical" and decided it must be better. Since java classes are exposed in JRuby, this demonstrates the difference

jruby-1.7.22 :003 > f = java.io.File.new('./foobar')
 => #<Java::JavaIo::File:0x4e70a141> 
jruby-1.7.22 :004 > f.getCanonicalPath()
 => "/home/ericu/running_as" 
jruby-1.7.22 :005 > f.getPath()
 => "./foobar" 
jruby-1.7.22 :006 > f.getAbsolutePath()
 => "/home/ericu/./foobar" 

Calling getAbsolutePath() or getPath() should provide the correct answer.

I monkey patched around this problem so I could get JRuby 1.7.22 into the production environment at my employer.

@enebo enebo modified the milestones: JRuby 1.7.26, JRuby 1.7.25 Apr 14, 2016
@enebo
Copy link
Member

enebo commented Apr 14, 2016

@hydrogen18 we will try and make sure we prioritize this for the next point release but we have some pressure to get 1.7.25 out so I am punting. Thanks for the repro. This will make it much easier for us to solve this problem.

I suspect your canonical theory is not correct. At least, traditionally, we have used canonical specifically detect softlinks prior to nio.2. So we have went through a lot of pain trying to detect stuff Java did not easily allow. I suspect our messy proc launching code somehow got changed and we ended up doing down a different code path. Process-launching has been one of our hardest things to support over the years and 1.7.x in particular shows its age. In 9k, the new native path is pretty much what MRI does now.

@enebo enebo modified the milestones: JRuby 1.7.26, JRuby 1.7.27 Aug 26, 2016
@hydrogen18
Copy link
Author

Is this still a bug in the latest 1.7 release?

@headius
Copy link
Member

headius commented Jan 26, 2017

@hydrogen18 This could be really simple. We're probably just passing the long path for argv[0] when we should be passing the unresolved path, as @hydrogen18 pointed out above. I'll have a quick look.

@headius
Copy link
Member

headius commented Jan 26, 2017

Doesn't look like anyone has tried 9k, so I can confirm it works correctly:

~/projects/jruby $ cat blah.c
#include "stdio.h"

int main(int argc, char** argv) {
  printf(argv[0]);
  return 0;
}

~/projects/jruby $ jruby -e 'p `./blah`'
"./blah"

~/projects/jruby $ jruby -e 'p `./blah_link`'
"./blah_link"

@hydrogen18 You are trying to use 9k now, right? I mean, this still could be fixed in 1.7, but we're not really maintaining that line anymore.

@hydrogen18
Copy link
Author

Yes actually I was just trying to figure out if I needed our monkey patch in the 1.7 branch.

@headius
Copy link
Member

headius commented Mar 15, 2017

@hydrogen18 I don't know actually. We are looking at doing a 1.7.27 to wrap up that line, but I'm not sure this needs to be fixed...and if it does I'm not sure we'll bother since it works properly in 9k.

@enebo enebo modified the milestones: Won't Fix, JRuby 1.7.27 Apr 24, 2017
@enebo
Copy link
Member

enebo commented Apr 24, 2017

EOL'ing 1.7.x after our next release and no one is stepping up for this.

@enebo enebo closed this as completed Apr 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants