4 Replies Latest reply on Feb 13, 2004 1:20 AM by Juha Lindfors

    ModelMBeanAttributeInterceptor flaw

    Scott Stark Master

      I'm testing out an authentication interceptor that would be applied globally to all operations, and I'm seeing that an Invocation of type InvocationContext.OP_SETATTRIBUTE is transformed into another Invocation of type InvocationContext.OP_INVOKE for the set method dispatch:

      invoke():81, org.jboss.security.jmx.AuthenticationInterceptor, AuthenticationInterceptor.java
      invoke():72, org.jboss.mx.server.Invocation, Invocation.java
      invoke():187, org.jboss.mx.server.AbstractMBeanInvoker, AbstractMBeanInvoker.java
      invoke():70, org.jboss.mx.interceptor.ModelMBeanAttributeInterceptor, ModelMBeanAttributeInterceptor.java
      invoke():70, org.jboss.mx.interceptor.PersistenceInterceptor, PersistenceInterceptor.java
      invoke():81, org.jboss.security.jmx.AuthenticationInterceptor, AuthenticationInterceptor.java
      invoke():72, org.jboss.mx.server.Invocation, Invocation.java
      setAttribute():376, org.jboss.mx.server.AbstractMBeanInvoker, AbstractMBeanInvoker.java
      setAttribute():462, org.jboss.mx.server.MBeanServerImpl, MBeanServerImpl.java
      setAttribute():485, org.jboss.system.ServiceConfigurator, ServiceConfigurator.java

      The relevant 3.2 branch code block from ModelMBeanAttributeInterceptor being:

       MBeanInvoker invoker = invocation.getInvoker();
      
       if (setMethod != null)
       {
       // TODO: should investigate chaining invocations rather than
       // going back to the invoker (JPL)
      
       // if setter was found, invoke the corresponding setter operation
       try
       {
       invoker.invoke(setMethod, new Object[] { value }, new String[] { invocation.getAttributeType() });
       }
       catch (Throwable t)
       {
       throw new InvocationException(t);
       }
       }
      


      This ends up sending the call back through the interceptor stack as an invocation only to exhaust the interceptor stack so that the dispatcher is called. This mixture of interceptors, invokers and dispatchers is complicating the insertion of aspects into the call chain.

      I don't see why its doing this new invocation, it should be calling the dispatcher directly here.



        • 1. Re: ModelMBeanAttributeInterceptor flaw
          Juha Lindfors Master

          setter chain is the mbean.setAttribute() chain, this however can be mapped to an operation in an MBean -- so you have for instance

          setAttribute("Foo")

          mapped to operation "readFoo" rather than the JavaBean setFoo() convention.

          I redirected the invocation back to the top of the "readFoo" chain in case a) there's a security check that needs to be performed on operation "readFoo" and don't want the setAttribute chain to accidentally bypass this b) there's some semantic in "readFoo" interceptors that should be executed as part of the setAttribute() call.

          • 2. Re: ModelMBeanAttributeInterceptor flaw
            Juha Lindfors Master

            The comment on TODO in the code points out that an invocation itself should be a Dispather interface implementation, and therefore invocations could be chained (where in this case the setAttribute()'s dispatcher would in reality be another Invocation with any additional logic needed to execute the operation that performs the "set"), rather than going back up to the invoker.

            • 3. Re: ModelMBeanAttributeInterceptor flaw
              Scott Stark Master

              I get the need to have the dispatch of the logical setAttribute("Foo") to some method other than setFoo on the resource, but the Invocation type is not staying consistent with the invocation coming from the server. The use case I'm working with is to set an authentication interceptor globally that only handles operations originating from the MBeanServer invoke call. The invocation of the 'notSetFoo' operation to actually set the 'Foo' attribute still should have type InvocationContext.OP_SETATTRIBUTE.

              However, since an Invocation is already a dispatcher, what I would like to see in the ModelMBeanAttributeInterceptor invoke implementation is:

               if (setMethod != null)
               {
               // if setter was found, invoke the corresponding setter operation
               try
               {
               invocation.dispatch();
               }
               catch (Throwable t)
               {
               throw new InvocationException(t);
               }
               }
              


              With the attribute InvocationContexts setup correctly to have the method
              dispatcher. Do you see a problem in making this change?


              • 4. Re: ModelMBeanAttributeInterceptor flaw
                Juha Lindfors Master

                 

                "starksm" wrote:
                The invocation of the 'notSetFoo' operation to actually set the 'Foo' attribute still should have type InvocationContext.OP_SETATTRIBUTE.


                Agreed.

                "starksm" wrote:

                However, since an Invocation is already a dispatcher, what I would like to see in the ModelMBeanAttributeInterceptor invoke implementation is:

                 if (setMethod != null)
                 {
                 // if setter was found, invoke the corresponding setter operation
                 try
                 {
                 invocation.dispatch();
                 }
                 catch (Throwable t)
                 {
                 throw new InvocationException(t);
                 }
                 }
                


                With the attribute InvocationContexts setup correctly to have the method
                dispatcher. Do you see a problem in making this change?


                Don't see a problem. That was the original intention with Invocation acting as a dispatcher.