Exclude User Destination Messages in Broker Message Handler

[ellers-espirit]

Affects: v5.3.3

We used WebSockets within Spring Boot 2.3.5 (Spring Framework 5.2.10). After updating to Spring Boot 2.4.2 (Spring Framework 5.3.3), we noticed that our WebSocket mechanism was no longer working.

We looked at #26118, but the discussion there was about duplicate subscriptions and wrongly registered /user destinations. In our case we have only a single subscription per sessionId/subscriptionId.

WebSocket is basically configured like this:

@Configuration
@EnableWebSocketMessageBroker
public class WebSocketConfig implements WebSocketMessageBrokerConfigurer {
	@Override
	public void configureMessageBroker(final MessageBrokerRegistry registry) {
		registry.setApplicationDestinationPrefixes("/app");
		registry.setUserDestinationPrefix("/user");
	}
	@Override
	public void registerStompEndpoints(final StompEndpointRegistry registry) {
		registry.addEndpoint("/websocket").withSockJS();
	}
}

In our application we use user-specific WebSocket messages as a push mechanism. The client subscribes to /user/events, the server sends messages to /user/{username}/events using SimpMessagingTemplate#convertAndSendToUser. This worked as expected up to Version 5.3.3.

_simpMessagingTemplate.convertAndSendToUser("username", "/events", pojo);

After a little bit of debugging, we've noticed that there might now be a problem with user-specific subscriptions due to a change with #24395. DestinationCache#calculateSubscriptions now operates on an exact destination equals-comparison when using non-pattern-based subscriptions.

		@NonNull
		private LinkedMultiValueMap<String, String> calculateSubscriptions(String destination) {
			LinkedMultiValueMap<String, String> sessionsToSubscriptions = new LinkedMultiValueMap<>();
			DefaultSubscriptionRegistry.this.subscriptionRegistry.forEachSubscription((sessionId, subscriptionDetail) -> {
				if (subscriptionDetail.isAntPattern()) {
					...
				}
				else if (destination.equals(subscriptionDetail.getDestination())) {
					...

While debugging the issue you can see, that the active subscription to subscriptionDetail.getDestination()="/user/events" is compared to a given destination="/user/{username}/events", which has been extracted from the new user-specific Message in AbstractSubscriptionRegistry#findSubscriptions called by SimpleBrokerMessageHandler#sendMessageToSubscribers.

To me it looks like the {username} part of the message destination is not properly handled here.

[rstoyanchev]

registry.setApplicationDestinationPrefixes("/app");
registry.setUserDestinationPrefix("/user");

Can you share the destinations to which the message broker is configured to process? The problem in #26118 is not that there are duplicate subscriptions, but that the user destinations are processed by both the broker and the user destination handler. That was explained in my last #26118 (comment) there.

[ellers-espirit]

You mean something like registry.enableSimpleBroker("/topic")? We've configured no specific destinations for the broker. The SimpleBrokerMessageHandler should be in it's default state. There was no need for any further configuration up until now.

We currently only use /user based unidirectional push messages through the aforementioned SimpMessagingTemplate, no client-initiated messages. Is there something we need to change using v5.3.3 regarding the WebSocket configuration?

I'm still confused about the inconsistent /user destinations in the DestinationCache at runtime (parameterized with {username}, registered without).

[rstoyanchev]

We've configured no specific destinations for the broker. The SimpleBrokerMessageHandler should be in it's default state.

So that means it's processing all destinations. This is essentially the same issue.

[rstoyanchev]

Once again "/user" prefixed messages aren't supposed to go directly to the broker. They are meant to be transformed by the user destination message handler first, which will then send them on the broker channel.

[ellers-espirit]

Shouldn't the broker configuration handle "/user" prefixed messages by default the way you describe? To me this looks like something the default configuration should be able to handle properly.

Maybe I have some kind of misunderstanding.. I'm not sure what the configuration should look like than. Do I need to specify every destination following after "/user/{username}", just to make sure the "/user" prefixed message are handled elsewhere, despite the fact that I configured setUserDestinationPrefix explicitly? That clearly has changed with v5.3.3 and seems more like a regression to me.

[ellers-espirit]

Btw, thanks for your last explanation about the separate user message handler. That explains the unexpected {username} destination part at that point. At least I can try to get it working again. :) Still think it is an unnecessary regression though ^^

[rstoyanchev]

Do I need to specify every destination following after "/user/{username}", just to make sure the "/user" prefixed message are handled elsewhere

There is a common convention in message brokers to use "/topic" or "/queue" as a prefix for pub-sub vs point-to-point messages. So the expectation is that you would use something to differentiate from application-bound (e.g. "/app") or direct user (i.e. "/user") messages.

That said I can see a potential improvement. When a user destination prefix is configured, then the broker message handler should exclude such messages from its processing.