fix: handle null password in MqttContext.isValid to avoid NPE - #6924
Open
wy471x wants to merge 4 commits into
Open
fix: handle null password in MqttContext.isValid to avoid NPE#6924wy471x wants to merge 4 commits into
wy471x wants to merge 4 commits into
Conversation
CONNECT with password flag 0 yields a null passwordInBytes, which caused new String((byte[]) null) to throw and hang the connection without a CONNACK. Treat null as empty so the client receives CONNECTION_REFUSED_BAD_USER_NAME_OR_PASSWORD, and add unit tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Aias00
approved these changes
Aug 17, 2026
Aias00
left a comment
Contributor
There was a problem hiding this comment.
Review: #6924 fix NPE in MqttContext.isValid on null passwordInBytes
Decision: APPROVE
What changed
MqttContext.isValid(...): whenpasswordInBytesisnull(CONNECT with password flag bit = 0, i.e. anonymous / username-only clients),new String((byte[]) null)previously threw NPE and killed the handler thread, so the client never got a CONNACK and the connection hung. Nownullis treated as empty string viaObjects.isNull(passwordInBytes) ? "" : new String(passwordInBytes), so validation fails gracefully withCONNECTION_REFUSED_BAD_USER_NAME_OR_PASSWORD.- Added
import java.util.Objects;. - Added
junit-jupitertest dependency (test scope) andMqttContextTestwith 6 cases (correct creds, null/empty password, wrong password, null/empty username).
Verification
Objectsimport is added in the same class, so it compiles.- The
nullbranch is handled beforenew String(...), eliminating the NPE path. - Test covers both the null and empty-password cases.
Notes
- Clean, minimal, spec-reasonable fix. No behavioral side effects for valid clients.
Good work — approving.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CONNECT with password flag 0 yields a null passwordInBytes, which caused new String((byte[]) null) to throw and hang the connection without a CONNACK. Treat null as empty so the client receives CONNECTION_REFUSED_BAD_USER_NAME_OR_PASSWORD, and add unit tests.
Make sure that:
./mvnw clean install -Dmaven.javadoc.skip=true.Summary
Changes:
username-only clients). The handler thread dies and the client never receives a CONNACK, leaving the connection hanging.
Test Cases:
close #6847