[BUG] TopicUtil.isValidTopicFilter accepts "$share/g/" — a shared subscription with an empty filter that can never match
@efegokdemir is already working on this.
Since Sep 24, 2026.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 75/100
Research direction
The bug is in the file bifromq-util/src/main/java/org/apache/bifromq/util/TopicUtil.java, specifically the isValidTopicFilter method. Start by reading the method around lines 74-161 to understand the validation logic for shared subscriptions. The issue is that a filter like "$share/g/" passes validation incorrectly. Add a check after incrementing i to ensure the filter part is non-empty. Run existing tests to verify the fix does not break other cases.
Written by the indexing model from the issue text.
Description
Environment
- BiFroMQ
4.0.0 - Verified against
main@ba2c34e0— still present on the latestmainat the time of filing - Module:
bifromq-util - File:
bifromq-util/src/main/java/org/apache/bifromq/util/TopicUtil.java:74-161
Summary
isValidTopicFilter validates the share name and then relies on the main scan loop to validate the real topic filter. When the filter part is empty ("$share/g/"), the loop body never executes and the method returns true.
Root cause
if (topicFilter.startsWith(PREFIX_ORDERED_SHARE) || topicFilter.startsWith(PREFIX_UNORDERED_SHARE)) {
// validate share name
for (i = topicFilter.indexOf(DELIMITER_CHAR) + 1; i < topicFilter.length(); i++) { ... }
if (topicLevelLength == 0) {
return false; // [MQTT-4.8.2-1]
}
if (i == topicFilter.length()) {
return false; // [MQTT-4.8.2-2]
}
topicLevelLength = 0;
// skip one separator to real topicFilter start pos
i++; // :112
}
int startIdx = i;
int level = 1;
for (; i < topicFilter.length(); i++) { // :116 body never runs when i == length
...
}
...
return topicLevelLength <= maxLevelLength; // :161 0 <= maxLevelLength -> true
The two guards at :102 and :106 only constrain the share name. Nothing checks that the filter part is non-empty. For "$share/g/", i is advanced to topicFilter.length(), so the loop at :116 is skipped entirely and the method falls through to the final return.
Verification
Trace for "$share/g/" (length 9):
| Step | Value |
|---|---|
i = indexOf('/') + 1 |
7 |
scan share name, break on '/' at index 8 |
topicLevelLength = 1 → passes :102 |
i (8) == length (9)? |
no → passes :106 |
i++ → |
9 |
for (; i < 9; i++) |
body not executed |
| final return | topicLevelLength (0) <= maxLevelLength → true |
Probing the compiled classes confirms isValidTopicFilter("$share/g/", ...) == true while "$share/g", "$share//t", "$share/" and "$share/+/t" are all correctly rejected. "$oshare/g/" behaves the same.
The neighbouring cases having been rejected correctly is what makes this a plain validation gap rather than an intentional relaxation.
Impact
"$share/g/" passes every gate in MQTTSessionHandler.checkAndSubscribe (isWellFormed → true, isValidTopicFilter → true, isWildcardTopicFilter → false), so:
SUBACKis returned with success,- a topic-filter slot and a shared-subscription route are consumed,
MqttSharedSubNumGaugeis incremented,
while the effective filter is the empty string. TopicUtil.from("$share/g/") yields filterLevel=[<empty level>], and the only topic that expands to an empty level is "" — which is itself invalid (TopicUtil.isValidTopic("") == false). No publishable topic can ever match, so the subscriber never receives a message.
Suggested fix
Require the filter part to be non-empty immediately after skipping the separator:
topicLevelLength = 0;
// skip one separator to real topicFilter start pos
i++;
if (i >= topicFilter.length()) {
return false; // "$share/g/" has an empty topic filter
}
}
- Dominant language
- Java
- Stars
- 805
- Forks
- 99
- Avg merge
- 16h 32m
- Merged PRs (30d)
- 2
Getting set up
- Ships a Dockerfile or Docker Compose file
- No pull request template
- No contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from apache/bifromq
-
[BUG] KVRangeRestorer leaves the restore future uncompleted when startRestore() throws, permanently wedging the rangePossibly taken @ImDanXie claimed this 32 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
[BUG] UTF8Util.isWellFormed inverts the surrogate-pair check: valid non-BMP characters are rejected, lone surrogates acceptedPossibly taken @efegokdemir claimed this 34 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
-
Difficulty 4/5 3-5 days Newbie friendliness 30/100
-
Inbound and outbound packet-size checks differ, so a QoS 1 message just under MaxUserPayloadBytes is accepted but never delivered (blocks a persistent subscriber)May be free again @abhiramaab claimed this 26 days ago, and no pull request is open. Open
Difficulty 4/5 3-5 days Newbie friendliness 55/100
-
Difficulty 4/5 3-5 days Newbie friendliness 48/100
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 80/100
-
DogFooding
Difficulty 1/5 Under an hour Newbie friendliness 88/100
OpenAEV-Platform/openaev#8768 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
federicoiosue/Omni-Notes#1039 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Dans-Plugins/Medieval-Roleplay-Engine#384 ·
Maintainers usually reply within 1 day
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Maintainers usually reply within 1 day