Repository: karaf Updated Branches: refs/heads/master 9bfa4aaaf -> 1fff01319
[KARAF-4523] JMXSecurityMBean bulk canInvoke should be robust even if bulkQuery contains duplicate operations This fix is based on the original patch by Grzegorz Grzybek Project: http://git-wip-us.apache.org/repos/asf/karaf/repo Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/1fff0131 Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/1fff0131 Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/1fff0131 Branch: refs/heads/master Commit: 1fff0131967ac11b0a481f503fbc952f777fc30d Parents: 9bfa4aa Author: Tadayoshi Sato <[email protected]> Authored: Tue May 10 11:32:34 2016 +0900 Committer: Tadayoshi Sato <[email protected]> Committed: Tue May 10 11:32:34 2016 +0900 ---------------------------------------------------------------------- management/server/pom.xml | 6 +++ .../internal/JMXSecurityMBeanImpl.java | 12 +++++- .../internal/JMXSecurityMBeanImplTestCase.java | 43 ++++++++++++++++++++ .../server/src/test/resources/log4j.properties | 34 ++++++++++++++++ 4 files changed, 94 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/karaf/blob/1fff0131/management/server/pom.xml ---------------------------------------------------------------------- diff --git a/management/server/pom.xml b/management/server/pom.xml index 19a5f28..2600900 100644 --- a/management/server/pom.xml +++ b/management/server/pom.xml @@ -75,6 +75,12 @@ <artifactId>slf4j-api</artifactId> <scope>provided</scope> </dependency> + + <dependency> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-log4j12</artifactId> + <scope>test</scope> + </dependency> </dependencies> <build> http://git-wip-us.apache.org/repos/asf/karaf/blob/1fff0131/management/server/src/main/java/org/apache/karaf/management/internal/JMXSecurityMBeanImpl.java ---------------------------------------------------------------------- diff --git a/management/server/src/main/java/org/apache/karaf/management/internal/JMXSecurityMBeanImpl.java b/management/server/src/main/java/org/apache/karaf/management/internal/JMXSecurityMBeanImpl.java index 683be51..3cde22c 100644 --- a/management/server/src/main/java/org/apache/karaf/management/internal/JMXSecurityMBeanImpl.java +++ b/management/server/src/main/java/org/apache/karaf/management/internal/JMXSecurityMBeanImpl.java @@ -18,6 +18,8 @@ package org.apache.karaf.management.internal; import org.apache.karaf.management.JMXSecurityMBean; import org.apache.karaf.management.KarafMBeanServerGuard; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.management.MBeanServer; import javax.management.NotCompliantMBeanException; @@ -25,6 +27,7 @@ import javax.management.ObjectName; import javax.management.StandardMBean; import javax.management.openmbean.CompositeData; import javax.management.openmbean.CompositeDataSupport; +import javax.management.openmbean.KeyAlreadyExistsException; import javax.management.openmbean.TabularData; import javax.management.openmbean.TabularDataSupport; import java.util.ArrayList; @@ -33,6 +36,8 @@ import java.util.Map; public class JMXSecurityMBeanImpl extends StandardMBean implements JMXSecurityMBean { + private static final Logger LOG = LoggerFactory.getLogger(JMXSecurityMBeanImpl.class); + private MBeanServer mbeanServer; private KarafMBeanServerGuard guard; @@ -99,7 +104,12 @@ public class JMXSecurityMBeanImpl extends StandardMBean implements JMXSecurityMB res = canInvoke(context, objectName, name, argTypes.toArray(new String[]{})); } CompositeData data = new CompositeDataSupport(CAN_INVOKE_RESULT_ROW_TYPE, CAN_INVOKE_RESULT_COLUMNS, new Object[]{ objectName, method, res }); - table.put(data); + try { + table.put(data); + } catch (KeyAlreadyExistsException e) { + // KeyAlreadyExistsException can happen only when methods are not empty + LOG.warn("{} (objectName = \"{}\", method = \"{}\")", e, objectName, method); + } } } } http://git-wip-us.apache.org/repos/asf/karaf/blob/1fff0131/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java ---------------------------------------------------------------------- diff --git a/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java b/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java index c77c484..6530156 100644 --- a/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java +++ b/management/server/src/test/java/org/apache/karaf/management/internal/JMXSecurityMBeanImplTestCase.java @@ -200,6 +200,49 @@ public class JMXSecurityMBeanImplTestCase extends TestCase { assertEquals(false, cd5.get("CanInvoke")); } + public void testCanInvokeBulkWithDuplicateMethods() throws Exception { + MBeanServer mbs = EasyMock.createMock(MBeanServer.class); + EasyMock.replay(mbs); + + ConfigurationAdmin testConfigAdmin = EasyMock.createMock(ConfigurationAdmin.class); + EasyMock.expect(testConfigAdmin.listConfigurations(EasyMock.eq("(service.pid=jmx.acl*)"))) + .andReturn(new Configuration[0]).anyTimes(); + EasyMock.expect(testConfigAdmin.listConfigurations(EasyMock.eq("(service.pid=jmx.acl.whitelist)"))) + .andReturn(new Configuration[0]).once(); + EasyMock.replay(testConfigAdmin); + + KarafMBeanServerGuard testGuard = EasyMock.createMock(KarafMBeanServerGuard.class); + String objectName = "foo.bar.testing:type=SomeMBean"; + final String[] la = new String[]{"long"}; + final String[] sa = new String[]{"java.lang.String"}; + EasyMock.expect(testGuard.getConfigAdmin()).andReturn(testConfigAdmin).anyTimes(); + EasyMock.expect(testGuard.canInvoke(EasyMock.anyObject(BulkRequestContext.class), EasyMock.eq(mbs), EasyMock.eq(new ObjectName(objectName)), EasyMock.eq("duplicateMethod1"), EasyMock.aryEq(la))).andReturn(true).anyTimes(); + EasyMock.expect(testGuard.canInvoke(EasyMock.anyObject(BulkRequestContext.class), EasyMock.eq(mbs), EasyMock.eq(new ObjectName(objectName)), EasyMock.eq("duplicateMethod1"), EasyMock.aryEq(sa))).andReturn(false).anyTimes(); + EasyMock.expect(testGuard.canInvoke(EasyMock.anyObject(BulkRequestContext.class), EasyMock.eq(mbs), EasyMock.eq(new ObjectName(objectName)), EasyMock.eq("duplicateMethod2"))).andReturn(true).anyTimes(); + EasyMock.replay(testGuard); + + JMXSecurityMBeanImpl mb = new JMXSecurityMBeanImpl(); + mb.setMBeanServer(mbs); + mb.setGuard(testGuard); + Map<String, List<String>> query = new HashMap<String, List<String>>(); + query.put(objectName, Arrays.asList("duplicateMethod1(long)", "duplicateMethod1(java.lang.String)", "duplicateMethod1(long)", "duplicateMethod2", "duplicateMethod2")); + TabularData result = mb.canInvoke(query); + assertEquals(3, result.size()); + + CompositeData cd = result.get(new Object[]{objectName, "duplicateMethod1(long)"}); + assertEquals(objectName, cd.get("ObjectName")); + assertEquals("duplicateMethod1(long)", cd.get("Method")); + assertEquals(true, cd.get("CanInvoke")); + CompositeData cd2 = result.get(new Object[]{objectName, "duplicateMethod1(java.lang.String)"}); + assertEquals(objectName, cd2.get("ObjectName")); + assertEquals("duplicateMethod1(java.lang.String)", cd2.get("Method")); + assertEquals(false, cd2.get("CanInvoke")); + CompositeData cd3 = result.get(new Object[]{objectName, "duplicateMethod2"}); + assertEquals(objectName, cd3.get("ObjectName")); + assertEquals("duplicateMethod2", cd3.get("Method")); + assertEquals(true, cd3.get("CanInvoke")); + } + public void testCanInvokeBulkCacheConfigAdmin() throws Exception { MBeanServer mbs = EasyMock.createMock(MBeanServer.class); EasyMock.replay(mbs); http://git-wip-us.apache.org/repos/asf/karaf/blob/1fff0131/management/server/src/test/resources/log4j.properties ---------------------------------------------------------------------- diff --git a/management/server/src/test/resources/log4j.properties b/management/server/src/test/resources/log4j.properties new file mode 100644 index 0000000..a75bae2 --- /dev/null +++ b/management/server/src/test/resources/log4j.properties @@ -0,0 +1,34 @@ +## --------------------------------------------------------------------------- +## Licensed to the Apache Software Foundation (ASF) under one or more +## contributor license agreements. See the NOTICE file distributed with +## this work for additional information regarding copyright ownership. +## The ASF licenses this file to You under the Apache License, Version 2.0 +## (the "License"); you may not use this file except in compliance with +## the License. You may obtain a copy of the License at +## +## http://www.apache.org/licenses/LICENSE-2.0 +## +## Unless required by applicable law or agreed to in writing, software +## distributed under the License is distributed on an "AS IS" BASIS, +## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +## See the License for the specific language governing permissions and +## limitations under the License. +## --------------------------------------------------------------------------- + +# +# The logging properties used during tests.. +# +log4j.rootLogger=INFO, console, file + +# Console will only display warnings +log4j.appender.console=org.apache.log4j.ConsoleAppender +log4j.appender.console.layout=org.apache.log4j.PatternLayout +log4j.appender.console.layout.ConversionPattern=%d{ISO8601} | %-5.5p | %-16.16t | %-32.32c{1} | %-32.32C %4L | %m%n +log4j.appender.console.threshold=WARN + +# File appender will contain all info messages +log4j.appender.file=org.apache.log4j.FileAppender +log4j.appender.file.layout=org.apache.log4j.PatternLayout +log4j.appender.file.layout.ConversionPattern=%d | %-5p | %m | %c | %t%n +log4j.appender.file.file=target/test.log +log4j.appender.file.append=true
