dotnet / dotnet/MQTTnet

Server subscriber sessions hash set can contain sessions with no subscriptions

Open
#2,062 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C#
Stars
5.1k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

### Describe the bug
In the server implementation, `MqttClientSessionsManager` keeps an internal hash set of "subscriber sessions" that have one or more subscriptions.

However, there is a bug that can result in this hash set containing sessions that have no subscriptions. If a subscribe message is intercepted and `ProcessSubscription = false` is set for all topic filters, then there will be no subscriptions to add. `MqttClientSubscriptionsManager.Subscribe` will still call `MqttClientSessionsManager.OnSubscriptionsAdded` with an empty `topics` list. This method will blindly add the session to `_subscriberSessions`.

https://github.com/dotnet/MQTTnet/blob/master/Source/MQTTnet/Server/Internal/MqttClientSubscriptionsManager.cs#L215
https://github.com/dotnet/MQTTnet/blob/master/Source/MQTTnet/Server/Internal/MqttClientSessionsManager.cs#L455

As a result, the server will need to check the session for subscriptions for every subsequent application message received which incurs some performance overhead for locking. There may also be other side effects due to this inconsistency.

### Which component is your bug related to?
Server

### To Reproduce
Steps to reproduce the behavior:
- Setup application to start up a server to intercept subscribe messages and set `ProcessSubscription = false`
- Confirm in debugger that session has been added to `_subscriberSessions`

### Expected behavior
Session should only be added to the `_subscriberSessions` if there was at least one subscription added. This then also avoids taking the `_sessionsManagementLock` when there is no work to do and avoids any other potential side effects.

```
if (addedSubscriptions.Count > 0)
{
_subscriptionChangedNotification?.OnSubscriptionsAdded(_session, addedSubscriptions);
}
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.