gtully commented on code in PR #5091:
URL: https://github.com/apache/activemq-artemis/pull/5091#discussion_r1687972386
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java:
##########
@@ -712,6 +712,19 @@ private synchronized void activate() throws Exception {
serverLocator.start(server.getExecutorFactory().getExecutor());
}
+ // add security role to disallow send/edit directly to sf queues
+ if (server.getConfiguration().isSecurityEnabled()) {
+ Set<Role> roles = new HashSet<>();
+ String addressMatch = storeAndForwardPrefix + name + "." +
server.getConfiguration().getWildcardConfiguration().getSingleWordString();
Review Comment:
I don't know that we want to add rbac to cluster connection in a special
way. My reference to rbac was in the context of a user using a producer to send
to the sfn, the assumption being that the cluster connection would be not being
doing this.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -1284,6 +1284,15 @@ public void addTail(final MessageReference ref, final
boolean direct) {
if (scheduleIfPossible(ref)) {
return;
}
+ if (checkInvalid(ref)) {
+ //send to dlq
+ try {
+ sendToDeadLetterAddress(null, ref);
Review Comment:
this looks sensible. Do we need to have some way to set a reason as to why
the message is on the DLQ? Or will it be implicit by peeking at the empty id
properties.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java:
##########
@@ -201,12 +201,6 @@ protected Message beforeForward(final Message message,
final SimpleString forwar
byte[] queueIds = message.getExtraBytesProperty(idsHeaderName);
- if (queueIds == null) {
- // Sanity check only
- ActiveMQServerLogger.LOGGER.noQueueIdDefined(message, messageCopy,
idsHeaderName);
Review Comment:
seems like routing to the DLQ here is the simplest solution.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -1284,6 +1284,15 @@ public void addTail(final MessageReference ref, final
boolean direct) {
if (scheduleIfPossible(ref)) {
return;
}
+ if (checkInvalid(ref)) {
+ //send to dlq
+ try {
+ sendToDeadLetterAddress(null, ref);
Review Comment:
do we need a new queue type and valid check, is is possible to just replace
the error AMQ222110 with a sent do dlq?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
