Apache
/
activemq-artemis
mirror of https://github.com/apache/activemq-artemis.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ARTEMIS-3058 improper AddressSettings merge 

The merge method in AddressSettings should *not* use any getters. It
should reference the relevant variables directly. Using any getters will
return default values in the underlying value is null. This can cause
problems for hierarchical settings.

Also fixed a few potential NPEs exposed by the test-case.
This commit is contained in:
Justin Bertram 2021-01-08 14:25:03 -06:00 committed by Clebert Suconic
parent 5dd147a023
commit 9aa7a10744
4 changed files with 43 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
View File

@ -1149,13 +1149,13 @@ public class PostOfficeImpl implements PostOffice, NotificationListener, Binding
// it's already been through here once, giving up now
sendToDLA = false;
} else {
sendToDLA = addressSettings.isSendToDLAOnNoRoute();
sendToDLA = addressSettings != null ? addressSettings.isSendToDLAOnNoRoute() : AddressSettings.DEFAULT_SEND_TO_DLA_ON_NO_ROUTE;
}
if (sendToDLA) {
// Send to the DLA for the address
SimpleString dlaAddress = addressSettings.getDeadLetterAddress();
SimpleString dlaAddress = addressSettings != null ? addressSettings.getDeadLetterAddress() : null;
if (logger.isDebugEnabled()) {
logger.debug("sending message to dla address = " + dlaAddress + ", message=" + message);
@ -1218,22 +1218,25 @@ public class PostOfficeImpl implements PostOffice, NotificationListener, Binding
// HORNETQ-1029
private void applyExpiryDelay(Message message, SimpleString address) {
long expirationOverride = addressSettingsRepository.getMatch(address.toString()).getExpiryDelay();
AddressSettings settings = addressSettingsRepository.getMatch(address.toString());
if (settings != null) {
long expirationOverride = settings.getExpiryDelay();
// A -1 <expiry-delay> means don't do anything
if (expirationOverride >= 0) {
// only override the expiration on messages where the expiration hasn't been set by the user
if (message.getExpiration() == 0) {
message.setExpiration(System.currentTimeMillis() + expirationOverride);
}
} else {
long minExpiration = addressSettingsRepository.getMatch(address.toString()).getMinExpiryDelay();
long maxExpiration = addressSettingsRepository.getMatch(address.toString()).getMaxExpiryDelay();
// A -1 <expiry-delay> means don't do anything
if (expirationOverride >= 0) {
// only override the expiration on messages where the expiration hasn't been set by the user
if (message.getExpiration() == 0) {
message.setExpiration(System.currentTimeMillis() + expirationOverride);
}
} else {
long minExpiration = settings.getMinExpiryDelay();
long maxExpiration = settings.getMaxExpiryDelay();
if (maxExpiration != AddressSettings.DEFAULT_MAX_EXPIRY_DELAY && (message.getExpiration() == 0 || message.getExpiration() > (System.currentTimeMillis() + maxExpiration))) {
message.setExpiration(System.currentTimeMillis() + maxExpiration);
} else if (minExpiration != AddressSettings.DEFAULT_MIN_EXPIRY_DELAY && message.getExpiration() < (System.currentTimeMillis() + minExpiration)) {
message.setExpiration(System.currentTimeMillis() + minExpiration);
if (maxExpiration != AddressSettings.DEFAULT_MAX_EXPIRY_DELAY && (message.getExpiration() == 0 || message.getExpiration() > (System.currentTimeMillis() + maxExpiration))) {
message.setExpiration(System.currentTimeMillis() + maxExpiration);
} else if (minExpiration != AddressSettings.DEFAULT_MIN_EXPIRY_DELAY && message.getExpiration() < (System.currentTimeMillis() + minExpiration)) {
message.setExpiration(System.currentTimeMillis() + minExpiration);
}
}
}
}

2
artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/AddressSettings.java
View File

@ -934,7 +934,7 @@ public class AddressSettings implements Mergeable<AddressSettings>, Serializable
pageMaxCache = merged.pageMaxCache;
}
if (pageSizeBytes == null) {
pageSizeBytes = merged.getPageSizeBytes();
pageSizeBytes = merged.pageSizeBytes;
}
if (messageCounterHistoryDayLimit == null) {
messageCounterHistoryDayLimit = merged.messageCounterHistoryDayLimit;

5
artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/Match.java
View File

@ -111,4 +111,9 @@ public class Match<T> {
}
}
@Override
public String toString() {
return value.toString();
}
}

18
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/AddressSettingsTest.java
View File

@ -284,6 +284,24 @@ public class AddressSettingsTest extends ActiveMQTestBase {
}
@Test
public void test3LevelHierarchyPageSizeBytes() throws Exception {
ActiveMQServer server = createServer(true);
server.start();
AddressSettings level1 = new AddressSettings().setPageSizeBytes(100 * 1024);
AddressSettings level2 = new AddressSettings();
AddressSettings level3 = new AddressSettings();
server.getAddressSettingsRepository().clear();
server.getAddressSettingsRepository().setDefault(null);
HierarchicalRepository<AddressSettings> repos = server.getAddressSettingsRepository();
repos.addMatch("test.foo.bar", level3);
repos.addMatch("test.foo.#", level2);
repos.addMatch("test.#", level1);
assertEquals(100 * 1024, server.getAddressSettingsRepository().getMatch("test.foo.bar").getPageSizeBytes());
}
@Test
public void testOverrideHierarchyWithDLA() throws Exception {
ActiveMQServer server = createServer(false);