enebo/jmx

MBeanServer#[] crashes in const_defined? when MBean class name contains $

Closed this issue · 5 comments

7er commented

The example shows what happens when trying to access a logback statistics appender bean
It's class name is:
com.yammer.metrics.reporting.JmxReporter$Meter
and that seems to be the reason for the following crash

jruby-1.6.7 :003 > client = JMX.connect(:port => 8999)
=> #<JMX::MBeanServer:0x32bed1fd @server=#<#Class:0x5b08ea49:0x62bc36ff>>
jruby-1.6.7 :004 > client['ch.qos.logback.core:type=Appender,name=all']
NameError: wrong constant name JmxReporter$Meter
from org/jruby/RubyModule.java:2561:in const_defined?' from /home/ses/.rvm/gems/jruby-1.6.7@production-simulator/gems/jmx-0.7/lib/jmx.rb:107:ingenerate'
from /home/ses/.rvm/gems/jruby-1.6.7@production-simulator/gems/jmx-0.7/lib/jmx/server.rb:44:in []' from (irb):4:inevaluate'
from org/jruby/RubyKernel.java:1088:in eval' from org/jruby/RubyKernel.java:1410:inloop'
from org/jruby/RubyKernel.java:1197:in catch' from org/jruby/RubyKernel.java:1197:incatch'
from /home/ses/.rvm/rubies/jruby-1.6.7/bin/jirb:17:in (root)' jruby-1.6.7 :005 > client['ch.qos.logback.core:type=Appender,name=error'] NameError: wrong constant name JmxReporter$Meter from org/jruby/RubyModule.java:2561:inconst_defined?'
from /home/ses/.rvm/gems/jruby-1.6.7@production-simulator/gems/jmx-0.7/lib/jmx.rb:107:in generate' from /home/ses/.rvm/gems/jruby-1.6.7@production-simulator/gems/jmx-0.7/lib/jmx/server.rb:44:in[]'
from (irb):5:in evaluate' from org/jruby/RubyKernel.java:1088:ineval'
from org/jruby/RubyKernel.java:1410:in loop' from org/jruby/RubyKernel.java:1197:incatch'
from org/jruby/RubyKernel.java:1197:in catch' from /home/ses/.rvm/rubies/jruby-1.6.7/bin/jirb:17:in(root)'

ouch...so we are storing our proxies as constants, but $ is invalid as a constant name. Perhaps we can mangle the name somehow (perhaps risk very unlikely collision of using _ in place of $ ... or dollar)?

7er commented

Yep, that is my understanding too. Some mangling seems to be in order. You already have a method in the MBeans module that does some mangling already if the class name starts with a lowercase letter. Both dollar and _ sounds good to me and would fix the problem. Maybe dollar would make it pretty obvious what has happended?

I'll see if I can submit a patch for this tomorrow

Great...I guess go for 'dollar' in the patch (we can easily change this if a better idea comes along).

7er commented

In jmx.rb there is a one line change commented below

  module MBeans
    ##
    # Create modules in this namespace for each package in the Java fully
    # qualified name and return the deepest module along with the Java class
    # name back to the caller.
    def self.parent_for(java_class_fqn)
      java_class_fqn.split(".").inject(MBeans) do |parent, segment|
        # const_defined? will crash later if we don't remove $
        segment.gsub!('$', 'Dollar') if segment =~ /\$/
        # Note: We are boned if java class name is lower cased
        return [parent, segment] if segment =~ /^[A-Z]/

        segment.capitalize!
        unless parent.const_defined? segment
          parent.const_set segment, Module.new
        else 
          parent.const_get segment
        end
      end

    end
  end

I created to 2 test cases to guide the patch, put them somewhere if they seem useful

  def test_that_mbeans_parent_for_handles_classes_with_dollar_sign
    bean_module, class_name = JMX::MBeans.parent_for("evil.dollar.Pre$Middle$Post")
    assert_equal(JMX::MBeans::Evil::Dollar, bean_module)
    assert_equal("PreDollarMiddleDollarPost", class_name)
  end

  def test_mbeans_parent_for
    bean_module, class_name = JMX::MBeans.parent_for("com.example.JavaClass")
    assert_equal(JMX::MBeans::Com::Example, bean_module)
    assert_equal("JavaClass", class_name)
  end

Applied this one-liner and also added these tests to jmx_mangling_test.rb. I put out a 0.8 with this change as well. Thanks for the patch and tests!