From ec2cb19f2d1b3b72c8e41bc9dc29537b44322eba Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Thu, 19 Nov 2020 15:48:18 -0600 Subject: [PATCH] ARTEMIS-3003 NPE when reloading persisted security-setting --- .../activemq/artemis/utils/DataConstants.java | 2 + .../config/PersistedSecuritySetting.java | 48 +++++++++++-------- .../management/ActiveMQServerControlTest.java | 4 +- .../PersistedSecuritySettingTest.java | 44 +++++++++++++++++ 4 files changed, 76 insertions(+), 22 deletions(-) create mode 100644 tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/PersistedSecuritySettingTest.java diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DataConstants.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DataConstants.java index 5b2b365ecc..ac2517c264 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DataConstants.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DataConstants.java @@ -34,6 +34,8 @@ public final class DataConstants { public static final int SIZE_CHAR = 2; + public static final int SIZE_NULL = 1; + public static final byte TRUE = 1; public static final byte FALSE = 0; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/config/PersistedSecuritySetting.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/config/PersistedSecuritySetting.java index f175be4249..f8eeb1f608 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/config/PersistedSecuritySetting.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/config/PersistedSecuritySetting.java @@ -20,6 +20,9 @@ import org.apache.activemq.artemis.api.core.ActiveMQBuffer; import org.apache.activemq.artemis.api.core.SimpleString; import org.apache.activemq.artemis.core.journal.EncodingSupport; +import static org.apache.activemq.artemis.utils.DataConstants.SIZE_INT; +import static org.apache.activemq.artemis.utils.DataConstants.SIZE_NULL; + public class PersistedSecuritySetting implements EncodingSupport { // Constants ----------------------------------------------------- @@ -116,74 +119,77 @@ public class PersistedSecuritySetting implements EncodingSupport { * @return the sendRoles */ public String getSendRoles() { - return sendRoles.toString(); + return sendRoles != null ? sendRoles.toString() : null; } /** * @return the consumeRoles */ public String getConsumeRoles() { - return consumeRoles.toString(); + return consumeRoles != null ? consumeRoles.toString() : null; } /** * @return the createDurableQueueRoles */ public String getCreateDurableQueueRoles() { - return createDurableQueueRoles.toString(); + return createDurableQueueRoles != null ? createDurableQueueRoles.toString() : null; } /** * @return the deleteDurableQueueRoles */ public String getDeleteDurableQueueRoles() { - return deleteDurableQueueRoles.toString(); + return deleteDurableQueueRoles != null ? deleteDurableQueueRoles.toString() : null; } /** * @return the createNonDurableQueueRoles */ public String getCreateNonDurableQueueRoles() { - return createNonDurableQueueRoles.toString(); + return createNonDurableQueueRoles != null ? createNonDurableQueueRoles.toString() : null; } /** * @return the deleteNonDurableQueueRoles */ public String getDeleteNonDurableQueueRoles() { - return deleteNonDurableQueueRoles.toString(); + return deleteNonDurableQueueRoles != null ? deleteNonDurableQueueRoles.toString() : null; } /** * @return the manageRoles */ public String getManageRoles() { - return manageRoles.toString(); + return manageRoles != null ? manageRoles.toString() : null; } /** * @return the browseRoles */ public String getBrowseRoles() { - return browseRoles.toString(); + return browseRoles != null ? browseRoles.toString() : null; } /** * @return the createAddressRoles */ public String getCreateAddressRoles() { - return createAddressRoles.toString(); + return createAddressRoles != null ? createAddressRoles.toString() : null; } /** * @return the deleteAddressRoles */ public String getDeleteAddressRoles() { - return deleteAddressRoles.toString(); + return deleteAddressRoles != null ? deleteAddressRoles.toString() : null; } @Override public void encode(final ActiveMQBuffer buffer) { + if (addressMatch == null) { + addressMatch = new SimpleString(""); + } buffer.writeSimpleString(addressMatch); buffer.writeNullableSimpleString(sendRoles); buffer.writeNullableSimpleString(consumeRoles); @@ -199,16 +205,18 @@ public class PersistedSecuritySetting implements EncodingSupport { @Override public int getEncodeSize() { - return addressMatch.sizeof() + SimpleString.sizeofNullableString(sendRoles) + - SimpleString.sizeofNullableString(consumeRoles) + - SimpleString.sizeofNullableString(createDurableQueueRoles) + - SimpleString.sizeofNullableString(deleteDurableQueueRoles) + - SimpleString.sizeofNullableString(createNonDurableQueueRoles) + - SimpleString.sizeofNullableString(deleteNonDurableQueueRoles) + - SimpleString.sizeofNullableString(manageRoles) + - SimpleString.sizeofNullableString(browseRoles) + - SimpleString.sizeofNullableString(createAddressRoles) + - SimpleString.sizeofNullableString(deleteAddressRoles); + return + (addressMatch == null ? SIZE_INT : addressMatch.sizeof()) + + (sendRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(sendRoles)) + + (consumeRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(consumeRoles)) + + (createDurableQueueRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(createDurableQueueRoles)) + + (deleteDurableQueueRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(deleteDurableQueueRoles)) + + (createNonDurableQueueRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(createNonDurableQueueRoles)) + + (deleteNonDurableQueueRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(deleteNonDurableQueueRoles)) + + (manageRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(manageRoles)) + + (browseRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(browseRoles)) + + (createAddressRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(createAddressRoles)) + + (deleteAddressRoles == null ? SIZE_NULL : SimpleString.sizeofNullableString(deleteAddressRoles)); } @Override diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java index 1f56b58520..f03914a571 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java @@ -904,7 +904,7 @@ public class ActiveMQServerControlTest extends ManagementTestBase { String exactAddress = "test.whatever"; assertEquals(1, serverControl.getRoles(addressMatch).length); - serverControl.addSecuritySettings(addressMatch, "foo", "foo, bar", "foo", "bar", "foo, bar", "", "", "bar", "foo", "foo"); + serverControl.addSecuritySettings(addressMatch, "foo", "foo, bar", null, "bar", "foo, bar", "", "", "bar", "foo", "foo"); // Restart the server. Those settings should be persisted @@ -926,7 +926,7 @@ public class ActiveMQServerControlTest extends ManagementTestBase { } assertTrue(fooRole.isSend()); assertTrue(fooRole.isConsume()); - assertTrue(fooRole.isCreateDurableQueue()); + assertFalse(fooRole.isCreateDurableQueue()); assertFalse(fooRole.isDeleteDurableQueue()); assertTrue(fooRole.isCreateNonDurableQueue()); assertFalse(fooRole.isDeleteNonDurableQueue()); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/PersistedSecuritySettingTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/PersistedSecuritySettingTest.java new file mode 100644 index 0000000000..1b60425bfe --- /dev/null +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/PersistedSecuritySettingTest.java @@ -0,0 +1,44 @@ +/* + * 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. + */ + +package org.apache.activemq.artemis.tests.integration.security; + +import org.apache.activemq.artemis.api.core.ActiveMQBuffer; +import org.apache.activemq.artemis.api.core.ActiveMQBuffers; +import org.apache.activemq.artemis.core.persistence.config.PersistedSecuritySetting; +import org.junit.Test; + +public class PersistedSecuritySettingTest { + + @Test + public void testNPE() { + PersistedSecuritySetting persistedSecuritySetting = new PersistedSecuritySetting(); + ActiveMQBuffer buffer = ActiveMQBuffers.fixedBuffer(persistedSecuritySetting.getEncodeSize()); + persistedSecuritySetting.encode(buffer); + persistedSecuritySetting.decode(buffer); + persistedSecuritySetting.getBrowseRoles(); + persistedSecuritySetting.getConsumeRoles(); + persistedSecuritySetting.getCreateAddressRoles(); + persistedSecuritySetting.getCreateDurableQueueRoles(); + persistedSecuritySetting.getCreateNonDurableQueueRoles(); + persistedSecuritySetting.getDeleteAddressRoles(); + persistedSecuritySetting.getDeleteDurableQueueRoles(); + persistedSecuritySetting.getDeleteNonDurableQueueRoles(); + persistedSecuritySetting.getManageRoles(); + persistedSecuritySetting.getSendRoles(); + } +}