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

You can crash irb by defining method `!` #4845

Open
aschmied opened this Issue Nov 9, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@aschmied

aschmied commented Nov 9, 2017

Defining a method ! causes a crash on !true.

Environment

Windows 10

> jruby --version
jruby 9.1.13.0 (2.3.3) 2017-09-06 8e1c115 Java HotSpot(TM) 64-Bit Server VM 25.144-b01 on 
1.8.0_144-b01 +jit [mswin32-x86_64]

> java -version
java version "1.8.0_144"
Java(TM) SE Runtime Environment (build 1.8.0_144-b01)
Java HotSpot(TM) 64-Bit Server VM (build 25.144-b01, mixed mode)

Actual Behaviour

> jirb
irb(main):001:0> def !
irb(main):002:1> end
=> :!
irb(main):003:0> !true
NoMethodError: private method `!' called for false:FalseClass
                               gets at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/input-method.rb:152
                block in eval_input at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb.rb:469
                      signal_status at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb.rb:623
                block in eval_input at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb.rb:468
                          buf_input at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/ruby-lex.rb:189
                               getc at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/ruby-lex.rb:104
                           match_io at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/slex.rb:206
                              match at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/slex.rb:76
                              token at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/ruby-lex.rb:283
                                lex at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/ruby-lex.rb:265
  block in each_top_level_statement at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/ruby-lex.rb:236
                               loop at org/jruby/RubyKernel.java:1292
  block in each_top_level_statement at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/ruby-lex.rb:232
                              catch at org/jruby/RubyKernel.java:1114
           each_top_level_statement at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb/ruby-lex.rb:231
                         eval_input at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb.rb:485
                     block in start at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb.rb:395
                              catch at org/jruby/RubyKernel.java:1114
                              start at C:/jruby/jruby-9.1.13.0/lib/ruby/stdlib/irb.rb:394
                             <main> at C:/jruby/jruby-9.1.13.0/bin/jirb:13
>

Expected Behavior

I don't know whether this is valid Ruby, but JRuby shouldn't crash

@enebo enebo changed the title from You can crash JRuby by defining method `!` to You can crash irb by defining method `!` Nov 9, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

The default visibility for methods defined in IRB is usually private, which is likely where this comes from. MRI doesn't produce an error, but it does basically kill IRB...

$ (chruby 2.4.1 ; irb)
irb(main):001:0> def !
irb(main):002:1> end
=> :!
irb(main):003:0> !true
irb(main):004:0> exit
irb(main):005:0> quit
irb(main):006:0> ^C
irb(main):006:0> 
[3]+  Stopped                 ( chruby 2.4.1; irb )

We seem to be defining this ! method differently than MRI, but in both cases it messes IRB up pretty good.

Member

headius commented Nov 9, 2017

The default visibility for methods defined in IRB is usually private, which is likely where this comes from. MRI doesn't produce an error, but it does basically kill IRB...

$ (chruby 2.4.1 ; irb)
irb(main):001:0> def !
irb(main):002:1> end
=> :!
irb(main):003:0> !true
irb(main):004:0> exit
irb(main):005:0> quit
irb(main):006:0> ^C
irb(main):006:0> 
[3]+  Stopped                 ( chruby 2.4.1; irb )

We seem to be defining this ! method differently than MRI, but in both cases it messes IRB up pretty good.

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Nov 9, 2017

Member

Ruby 2.3.3 also raises NoMethodError for this:

mri23 -e 'def !; end; !true'
-e:1:in `<main>': private method `!' called for true:TrueClass (NoMethodError)

Behaviorally we are correct at this but things get weird when done from within irb itself. It appears that the problem is that we replace ! globally while irb seems define this within a singleton instance? That is just a guess but the crash in irb while using JRuby is that since ! was defined then

          HISTORY.push(l) if !l.empty?

blows up.

Member

enebo commented Nov 9, 2017

Ruby 2.3.3 also raises NoMethodError for this:

mri23 -e 'def !; end; !true'
-e:1:in `<main>': private method `!' called for true:TrueClass (NoMethodError)

Behaviorally we are correct at this but things get weird when done from within irb itself. It appears that the problem is that we replace ! globally while irb seems define this within a singleton instance? That is just a guess but the crash in irb while using JRuby is that since ! was defined then

          HISTORY.push(l) if !l.empty?

blows up.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

This appears to only be different in IRB because in MRI top-level IRB methods get defined as public, where we define them as private:

[--dev] ~/projects/ruby $ (chruby 2.3.4 ; irb)
irb(main):001:0> def foo; end; self.class.public_instance_methods.sort.grep :foo
=> [:foo]
irb(main):002:0> exit

[--dev] ~/projects/ruby $ jirb
irb(main):001:0> def foo; end; self.class.public_instance_methods.sort.grep :foo
=> []
irb(main):002:0> 

I'm not sure why this is the case; we don't ship our own IRB.

Member

headius commented Nov 9, 2017

This appears to only be different in IRB because in MRI top-level IRB methods get defined as public, where we define them as private:

[--dev] ~/projects/ruby $ (chruby 2.3.4 ; irb)
irb(main):001:0> def foo; end; self.class.public_instance_methods.sort.grep :foo
=> [:foo]
irb(main):002:0> exit

[--dev] ~/projects/ruby $ jirb
irb(main):001:0> def foo; end; self.class.public_instance_methods.sort.grep :foo
=> []
irb(main):002:0> 

I'm not sure why this is the case; we don't ship our own IRB.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

MRI and JRuby both appear to set up the binding for IRB commands the same way, and from what I can tell these methods should be getting defined as private:

irb/workspace.rb

        when 3	# binding in function on TOPLEVEL_BINDING(default)
          @binding = eval("def irb_binding; private; binding; end; irb_binding",
                          TOPLEVEL_BINDING,
                          __FILE__,
                          __LINE__ - 3)

I do not know why MRI appears to define methods as public under this binding.

Member

headius commented Nov 9, 2017

MRI and JRuby both appear to set up the binding for IRB commands the same way, and from what I can tell these methods should be getting defined as private:

irb/workspace.rb

        when 3	# binding in function on TOPLEVEL_BINDING(default)
          @binding = eval("def irb_binding; private; binding; end; irb_binding",
                          TOPLEVEL_BINDING,
                          __FILE__,
                          __LINE__ - 3)

I do not know why MRI appears to define methods as public under this binding.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

It seems like under MRI, all method definitions at toplevel get public visibility, regardless of what current visibility is set to:

$ irb
:here
irb(main):001:0> private
=> Object
irb(main):002:0> def foo; end; self.class.public_instance_methods.grep :foo
=> [:foo]
irb(main):003:0> private def foo; end; self.class.public_instance_methods.grep :foo
=> []

I've filed a bug for this here: https://bugs.ruby-lang.org/issues/14094

Member

headius commented Nov 9, 2017

It seems like under MRI, all method definitions at toplevel get public visibility, regardless of what current visibility is set to:

$ irb
:here
irb(main):001:0> private
=> Object
irb(main):002:0> def foo; end; self.class.public_instance_methods.grep :foo
=> [:foo]
irb(main):003:0> private def foo; end; self.class.public_instance_methods.grep :foo
=> []

I've filed a bug for this here: https://bugs.ruby-lang.org/issues/14094

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

Note also that MRI "crashes" the same way if we force the ! method to be private:

irb(main):004:0> private def !; end
=> Object
irb(main):005:0> !false
/Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb/input-method.rb:152:in `gets': private method `!' called for false:FalseClass (NoMethodError)
	from /Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb.rb:469:in `block (2 levels) in eval_input'
	from /Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb.rb:623:in `signal_status'
	from /Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb.rb:468:in `block in eval_input'
	from /Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb/ruby-lex.rb:189:in `buf_input'

Workaround in JRuby (even though defining this ! breaks everything) is to set the method public at definition time, as I've set it private for MRI above.

I consider this a pretty low priority to fix.

@aschmied we'll see what MRI says about this. Perhaps they can clarify why they default to public.

Member

headius commented Nov 9, 2017

Note also that MRI "crashes" the same way if we force the ! method to be private:

irb(main):004:0> private def !; end
=> Object
irb(main):005:0> !false
/Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb/input-method.rb:152:in `gets': private method `!' called for false:FalseClass (NoMethodError)
	from /Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb.rb:469:in `block (2 levels) in eval_input'
	from /Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb.rb:623:in `signal_status'
	from /Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb.rb:468:in `block in eval_input'
	from /Users/headius/.rubies/ruby-2.3.4/lib/ruby/2.3.0/irb/ruby-lex.rb:189:in `buf_input'

Workaround in JRuby (even though defining this ! breaks everything) is to set the method public at definition time, as I've set it private for MRI above.

I consider this a pretty low priority to fix.

@aschmied we'll see what MRI says about this. Perhaps they can clarify why they default to public.

@aschmied

This comment has been minimized.

Show comment
Hide comment
@aschmied

aschmied Nov 10, 2017

Thanks, @headius. Feel free to close this. I was just experimenting because I came across an example of it in graphql-ruby.

aschmied commented Nov 10, 2017

Thanks, @headius. Feel free to close this. I was just experimenting because I came across an example of it in graphql-ruby.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 10, 2017

Member

@aschmied I'd like a good answer as to why MRI differs here, so I'll leave it open for now.

Member

headius commented Nov 10, 2017

@aschmied I'd like a good answer as to why MRI differs here, so I'll leave it open for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment