1 2 Previous Next 16 Replies Latest reply on Feb 7, 2007 5:34 AM by Ales Justin

    Property replacement

    Ales Justin Master

      Just so that we don't totally hijack this thread:
      - http://www.jboss.com/index.html?module=bb&op=viewtopic&t=99508

      I'm working on the issue.
      Just in the testing phase (having some problems with permission to set System property).

      I added this tests:
      - org.jboss.test.kernel.config.test.PropertyTestCase
      - org.jboss.test.kernel.config.test.PropertyXMLTestCase

        • 1. Re: Property replacement
          Ales Justin Master

          How do I get pass this:

          access denied (java.util.PropertyPermission test.property.value write)
          java.security.AccessControlException: access denied (java.util.PropertyPermission test.property.value write)
           at java.security.AccessControlContext.checkPermission(AccessControlContext.java:264)
           at java.security.AccessController.checkPermission(AccessController.java:427)
           at java.lang.SecurityManager.checkPermission(SecurityManager.java:532)
           at java.lang.System.setProperty(System.java:699)
           at org.jboss.test.kernel.config.test.PropertyTestCase$1.run(PropertyTestCase.java:64)
           at java.security.AccessController.doPrivileged(Native Method)
           at org.jboss.test.kernel.config.test.PropertyTestCase.testPropertyWithPropertyValue(PropertyTestCase.java:60)
           at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
           at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
           at junit.extensions.TestSetup.run(TestSetup.java:23)
          


          My code:
          public void testPropertyWithPropertyValue() throws Throwable
           {
           // set property to be replaced
           final String CONST = "PropertyReplaceTestCase";
          
           AbstractTestDelegate delegate = getDelegate();
           delegate.enableSecurity = false;
           AccessController.doPrivileged(new PrivilegedAction<Object>()
           {
           public Object run()
           {
           System.setProperty("test.property.value", CONST);
           return null;
           }
           });
          
           // get property
           Object value = instantiateReplacePropertyValue();
           assertNotNull(value);
           assertEquals(String.class, value.getClass());
           assertEquals(CONST, value);
           }
          


          • 2. Re: Property replacement
            Adrian Brock Master

            Hard to tell without seeing the whole code.

            I'd guess you have AbstractKernelTest somewhere in your hierarchy?

            Which installs a SecurityManager:

             public static AbstractTestDelegate getDelegate(Class clazz) throws Exception
             {
             AbstractTestDelegate delegate = new AbstractTestDelegate(clazz);
             delegate.enableSecurity = true;
             return delegate;
             }
            


            The tests get only very basic privilieges, the idea being that we want to make
            sure the MC works properly in a restricted environment.

            • 3. Re: Property replacement
              Adrian Brock Master

               

              "alesj" wrote:

              AbstractTestDelegate delegate = getDelegate();
              delegate.enableSecurity = false;


              This won't work.
              The flag is only considered during test setup

              You could perhaps add a couple of methods to AbstractTestCaseWithSetup
              that let you temporarily suspend the security manager.

              protected SecurityManager suspendSecurity()
              {
               SecurityManager result = System.getSecurityManager();
               System.setSecurityManager(null);
               return result;
              }
              
              protected void resumeSecurity(SecurityManager securityManager)
              {
               System.setSecurityManager(securityManager);
              }
              


              You could then do:

              SecurityManager sm = suspendSecurity();
              try
              {
               // Do priviledged stuff
              }
              finally
              {
               resumeSecurity(sm);
              }
              


              • 4. Re: Property replacement
                Adrian Brock Master

                Sorry that would of course need to be :-)

                protected SecurityManager suspendSecurity()
                {
                 return AccessController.doPrivileged(new PrivilegedAction<SecurityManager>
                 {
                 SecurityManager result = System.getSecurityManager();
                 System.setSecurityManager(null);
                 return result;
                 });
                }
                


                • 5. Re: Property replacement
                  Ales Justin Master

                  I added this SecurityManager suspend / resume and things get pass that write permission problem.

                  But now I see that the ${x} string gets actually picked up by simple StringEditor, and not by 'replacing' Editor.

                  I added this code check (in ValueConvertor):

                  if (clazz.isAssignableFrom(valueClass) && isPropertyReplaceValue(value) == false)
                   return value;
                  


                  So that we even get to PropertyEditors.

                  • 6. Re: Property replacement
                    Ales Justin Master

                     

                    "alesj" wrote:

                    But now I see that the ${x} string gets actually picked up by simple StringEditor, and not by 'replacing' Editor.


                    Ok, this is expected behaviour, since our targetType is String.

                    Should I change the StringEditor to look for replacement?
                    Or should I wrap the string value in some ReplecamentString class and add ReplecamentStringEditor into commons editors?

                    • 7. Re: Property replacement
                      Adrian Brock Master

                      None of the property editors should be doing system property replacement.

                      The best way to fix this and maintain backwards compatibility is the following
                      diff in ValueConvertor.

                      [ejort@warjort plugins]$ svn diff ValueConvertor.java
                      Index: ValueConvertor.java
                      ===================================================================
                      --- ValueConvertor.java (revision 59913)
                      +++ ValueConvertor.java (working copy)
                      @@ -31,6 +31,7 @@
                       import org.jboss.reflect.plugins.introspection.ReflectionUtils;
                       import org.jboss.reflect.spi.ProgressionConvertor;
                       import org.jboss.reflect.spi.ProgressionConvertorFactory;
                      +import org.jboss.util.StringPropertyReplacer;
                       import org.jboss.util.propertyeditor.PropertyEditors;
                      
                       /**
                      @@ -69,15 +70,37 @@
                       * @return the value or null if there is no editor
                       * @throws Throwable for any error
                       */
                      - @SuppressWarnings("unchecked")
                       public static Object convertValue(Class<? extends Object> clazz, Object value) throws Throwable
                       {
                      + return convertValue(clazz, value, false);
                      + }
                      +
                      + /**
                      + * Convert a value
                      + *
                      + * @param clazz the class
                      + * @param value the value
                      + * @param replaceProperties whether to replace system properties
                      + * @return the value or null if there is no editor
                      + * @throws Throwable for any error
                      + */
                      + @SuppressWarnings("unchecked")
                      + public static Object convertValue(Class<? extends Object> clazz, Object value, boolean replaceProperties) throws Throwable
                      + {
                       if (clazz == null)
                       throw new IllegalArgumentException("Null class");
                       if (value == null)
                       return null;
                      
                       Class<? extends Object> valueClass = value.getClass();
                      +
                      + // If we have a string replace any system properties when requested
                      + if (replaceProperties && valueClass == String.class)
                      + {
                      + String string = (String) value;
                      + value = StringPropertyReplacer.replaceProperties(string);
                      + }
                      +
                       if (clazz.isAssignableFrom(valueClass))
                       return value;
                      


                      Then add the following new method to TypeInfo
                      that invokes the alternative method in ValueConvertor

                       /**
                       * Convert a value
                       *
                       * @param value the original value
                       * @param replaceProperties whether to replace properties
                       * @return the converted value
                       * @throws Throwable for any error
                       */
                       Object convertValue(Object value, boolean replaceProperties) throws Throwable;
                      


                      The StringValueMetaData should be invoking this new method.

                      Finally, it would be good if StringValueMetaData also had an additional
                      parameter to turn off string property replace (like the JMX stuff does)

                      e.g. (but also for other places that take a "value")

                      <!-- This property is not replaced -->
                      <property name="blah" replace="false">${hello}</property>
                      


                      • 8. Re: Property replacement
                        Brian Stansberry Master

                        Apologies in advance if this is a hijack; I'll open another thread if the work discussed here isn't the cause...

                        In AS trunk with the latest microcontainer jars, this no longer works in a -beans.xml:

                        <annotation>@org.jboss.aop.microcontainer.aspects.jmx.JMX(name="jboss:service=HASingletonDeploymentScanner,partitionName=${jboss.partition.name:DefaultPartition}", exposedInterface=org.jboss.ha.singleton.HASingletonDeploymentScannerMBean.class, registerDirectly=true)</annotation>


                        You get:

                        10:18:00,181 ERROR [AbstractKernelController] Error installing to Configured: name=HASingletonDeploymentScanner state=Instantiated
                        javax.management.MalformedObjectNameException: Invalid character ':' in value part of property
                         at javax.management.ObjectName.construct(ObjectName.java:529)
                         at javax.management.ObjectName.<init>(ObjectName.java:1304)
                         at org.jboss.aop.microcontainer.aspects.jmx.JMXIntroduction.invoke(JMXIntroduction.java:71)
                         at org.jboss.aop.advice.org.jboss.aop.microcontainer.aspects.jmx.JMXIntroduction_z_invoke_24464082.invoke(JMXIntroduction_z_invoke_24464082.java)
                         at org.jboss.aop.joinpoint.MethodInvocation.invokeNext(MethodInvocation.java:101)
                         at AOPContainerProxy$5.setKernelControllerContext(AOPContainerProxy$5.java)
                         at org.jboss.kernel.plugins.dependency.KernelControllerContextAction.installAction(KernelControllerContextAction.java:208)
                         at org.jboss.kernel.plugins.dependency.KernelControllerContextAction.install(KernelControllerContextAction.java:136)
                        ...


                        • 9. Re: Property replacement
                          Ales Justin Master

                           

                          "bstansberry@jboss.com" wrote:

                          <annotation>@org.jboss.aop.microcontainer.aspects.jmx.JMX(name="jboss:service=HASingletonDeploymentScanner,partitionName=${jboss.partition.name:DefaultPartition}", exposedInterface=org.jboss.ha.singleton.HASingletonDeploymentScannerMBean.class, registerDirectly=true)</annotation>


                          Uf, yes.
                          I removed the XB doing the replacement, but forgot to add this in the annotation value handling - since this string value is directly used, and not via StringValueMetaData.

                          • 10. Re: Property replacement
                            Ales Justin Master

                            How do we change this:

                             <xsd:simpleType name="annotationType">
                             <xsd:annotation>
                             <xsd:documentation>
                             <![CDATA[
                             The annotation type represents a Java5 annotation on the particular join point.
                             ]]>
                             </xsd:documentation>
                             </xsd:annotation>
                             <xsd:restriction base="xsd:string">
                             <xsd:whiteSpace value="collapse"/>
                             </xsd:restriction>
                             </xsd:simpleType>
                            


                            so that I can add an this (replace="false"):
                            <annotation replace="false">@SomeAnnotation</annotation>


                            Is it ok that we have StringPropertyReplacer in AbstractAnnotationMetaData?
                            String annString = StringPropertyReplacer.replaceProperties(annotation);
                             //FIXME [JBMICROCONT-99] [JBAOP-278] Use the loader for the bean?
                             ann = (Annotation)AnnotationCreator.createAnnotation(annString, Thread.currentThread().getContextClassLoader());
                            


                            • 11. Re: Property replacement
                              Brian Stansberry Master

                              The places where I was using that syntax, it wasn't critical. So for now I've removed it so the AS trunk builds will work correctly. I'll monitor JBMICRONTAINER-143 and once it's done I'll restore the property replacement syntax in the annotation values.

                              Thanks!

                              • 12. Re: Property replacement
                                Scott Stark Master

                                 

                                "alesj" wrote:
                                How do we change this:
                                 <xsd:simpleType name="annotationType">
                                 <xsd:annotation>
                                 <xsd:documentation>
                                 <![CDATA[
                                 The annotation type represents a Java5 annotation on the particular join point.
                                 ]]>
                                 </xsd:documentation>
                                 </xsd:annotation>
                                 <xsd:restriction base="xsd:string">
                                 <xsd:whiteSpace value="collapse"/>
                                 </xsd:restriction>
                                 </xsd:simpleType>
                                



                                To allow an attribute on the annotation element, you need to change it to a complexType/simpleContent:

                                 <xsd:complexType name="annotationType">
                                 <xsd:annotation>
                                 <xsd:documentation>
                                 <![CDATA[
                                 The annotation type represents a Java5 annotation on the particular join point.
                                 ]]>
                                 </xsd:documentation>
                                 </xsd:annotation>
                                
                                 <xsd:simpleContent>
                                 <xsd:extension base="xsd:string">
                                 <xsd:attribute name="replace" type="xsd:boolean" default="false" />
                                 </xsd:extension>
                                 </xsd:simpleContent>
                                 </xsd:complexType>
                                


                                "alesj" wrote:

                                Is it ok that we have StringPropertyReplacer in AbstractAnnotationMetaData?
                                String annString = StringPropertyReplacer.replaceProperties(annotation);
                                 //FIXME [JBMICROCONT-99] [JBAOP-278] Use the loader for the bean?
                                 ann = (Annotation)AnnotationCreator.createAnnotation(annString, Thread.currentThread().getContextClassLoader());
                                


                                I guess, but I don't see this as a natural usage. A check of the replace attribute would be needed.


                                • 13. Re: Property replacement
                                  Ales Justin Master

                                   

                                  "scott.stark@jboss.org" wrote:

                                  I guess, but I don't see this as a natural usage. A check of the replace attribute would be needed.


                                  I agree.
                                  How can we move this in the same way as we did it with StringValueMD?
                                  But with this one is different - it's a plain String value with possible replacement.
                                  Own TypeInfo --> StringInfo (similar to NumberInfo)?

                                  • 14. Re: Property replacement
                                    Scott Stark Master

                                    In terms of the current schema, its just a different type of element that would need to be treated separately as the AnnotationMetaData does not have a natural string representation outside of the annotation attribute values. We would have to make this a richer type of element with more of a javabean type of property/value structure, but the allowed values are more restricted.

                                    There already is an AnnotationInfo. Doesn't the inherrited TypeInfo.convertValue(Object value, boolean replaceProperties) come into play?

                                    1 2 Previous Next