eclipse-paho / eclipse-paho/paho.mqtt.java

Long ClientId causes unwanted MqttPersistenceException inside MqttDefaultFilePersistence

Open
#1,061 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
2.3k
Forks
918
PR merge metrics
No merged PRs in 30d

Description

**Description:**

I encountered an issue in the `open` method where, if the combined length of `clientId` and `theConnection` exceeds the maximum file name length, the directory creation fails silently. This can cause the `restoreBackups` function to throw an `MqttPersistenceException`.

### Steps to Reproduce:
1. Pass a `clientId` and `theConnection` whose combined length (after filtering with `isSafeChar`) results in a `key` exceeding 255 characters.
2. When the `open` method tries to create a directory with this `key`, the directory creation fails because the file system's file name length limit is 255 characters.

### Impact:
This issue imposes an unintended limitation on MQTT's support for long clientId values, which should normally be supported up to 65,535 characters. It affects the persistence mechanism and could disrupt the normal operation of the library.

### Suggested Solutions:
1- **Validate key Length**: Add a check to ensure that the key length does not exceed 255 characters before creating the directory. If it does, consider truncating the key or using a hashed version to fit within the limit.
2- **Handle mkdir Failure**: Check the return value of mkdir and throw an appropriate exception if directory creation fails, with a clear error message indicating the cause.

### Relevant Code:
Here is the part of the `open` function where the issue occurs:
```java
if (!clientDir.exists()) {
clientDir.mkdir(); // Fails silently if the key length exceeds 255 characters
}
```
And in the `restoreBackups` method:
```java
File[] files = dir.listFiles(new PersistanceFileFilter(MESSAGE_BACKUP_FILE_EXTENSION));

if (files == null) {
throw new MqttPersistenceException();
}
```

However, if the length of the `key` exceeds 255 characters, `mkdir` fails and does not throw an exception. The directory remains non-existent, causing subsequent calls to `listFiles()` in the `restoreBackups` method to return `null`, which then triggers an `MqttPersistenceException`.

### Environment:
- Library version: 1.2.5

Contributor guide

Open the contributing guide

Research direction

Locate MqttDefaultFilePersistence and trace the open method into restoreBackups, including the PersistanceFileFilter call and the clientDir.mkdir result. Reproduce the failure with a key over 255 characters, then determine the agreed handling for directory creation and verify that long clientIds no longer lead to a misleading MqttPersistenceException.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.