This is an automated email from the ASF dual-hosted git repository.
dengliming pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new fb7180b87d fix: handle null password in MqttContext.isValid to avoid
NPE (#6924)
fb7180b87d is described below
commit fb7180b87d357aabcfa328c97f2271171e43bdc5
Author: wy471x <[email protected]>
AuthorDate: Fri Sep 18 11:23:47 2026 +0800
fix: handle null password in MqttContext.isValid to avoid NPE (#6924)
* fix: handle null password in MqttContext.isValid to avoid NPE
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 <[email protected]>
* test: simplify and parameterize MqttContextTest credential cases
The setUp method configured a throwaway MqttContext instance and then
overwrote the static state through another instance, so two assertions failed
and the result depended on execution order. Reuse the single test fixture, drop
duplicated isValid cases in favor of a parameterized source, and add coverage
for state sharing across instances and for re-configured credentials.
---------
Co-authored-by: Claude Opus 4.7 <[email protected]>
Co-authored-by: aias00 <[email protected]>
Co-authored-by: Liming Deng <[email protected]>
---
shenyu-protocol/shenyu-protocol-mqtt/pom.xml | 6 ++
.../apache/shenyu/protocol/mqtt/MqttContext.java | 4 +-
.../shenyu/protocol/mqtt/MqttContextTest.java | 114 +++++++++++++--------
3 files changed, 82 insertions(+), 42 deletions(-)
diff --git a/shenyu-protocol/shenyu-protocol-mqtt/pom.xml
b/shenyu-protocol/shenyu-protocol-mqtt/pom.xml
index 2ddc80e0af..cf8e31fb56 100644
--- a/shenyu-protocol/shenyu-protocol-mqtt/pom.xml
+++ b/shenyu-protocol/shenyu-protocol-mqtt/pom.xml
@@ -46,6 +46,12 @@
<artifactId>reflections</artifactId>
<version>0.9.11</version>
</dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git
a/shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/MqttContext.java
b/shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/MqttContext.java
index 74e6b0dd6a..7ecaf06856 100644
---
a/shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/MqttContext.java
+++
b/shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/MqttContext.java
@@ -19,6 +19,8 @@ package org.apache.shenyu.protocol.mqtt;
import org.apache.commons.lang3.StringUtils;
+import java.util.Objects;
+
/**
* mqtt env.
*/
@@ -45,7 +47,7 @@ public class MqttContext {
* @return true is correct, false unavailable.
*/
public static boolean isValid(final String userName, final byte[]
passwordInBytes) {
- String password = new String(passwordInBytes);
+ String password = Objects.isNull(passwordInBytes) ? "" : new
String(passwordInBytes);
if (StringUtils.isEmpty(password) || StringUtils.isEmpty(userName)) {
return false;
diff --git
a/shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/MqttContextTest.java
b/shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/MqttContextTest.java
index 6575362d6c..296c19f015 100644
---
a/shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/MqttContextTest.java
+++
b/shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/MqttContextTest.java
@@ -20,78 +20,110 @@ package org.apache.shenyu.protocol.mqtt;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.nio.charset.StandardCharsets;
+import java.util.stream.Stream;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
/**
* Test cases for {@link MqttContext}.
*/
public final class MqttContextTest {
+ private static final int PORT = 1883;
+
+ private static final int BOSS_GROUP_THREAD_COUNT = 1;
+
+ private static final int WORKER_GROUP_THREAD_COUNT = 2;
+
+ private static final int MAX_PAYLOAD_SIZE = 1024;
+
+ private static final String USER_NAME = "test-user";
+
+ private static final String PASSWORD = "test-password";
+
+ private static final String LEAK_DETECTOR_LEVEL = "disabled";
+
+ private static final byte[] PASSWORD_IN_BYTES =
PASSWORD.getBytes(StandardCharsets.UTF_8);
+
+ private final MqttContext mqttContext = new MqttContext();
+
@BeforeEach
public void setUp() {
- MqttContext context = new MqttContext();
- context.setPort(1883);
- context.setBossGroupThreadCount(1);
- context.setWorkerGroupThreadCount(2);
- context.setMaxPayloadSize(1024);
- context.setUserName("test-user");
- context.setPassword("test-password");
- context.setLeakDetectorLevel("disabled");
+ mqttContext.setPort(PORT);
+ mqttContext.setBossGroupThreadCount(BOSS_GROUP_THREAD_COUNT);
+ mqttContext.setWorkerGroupThreadCount(WORKER_GROUP_THREAD_COUNT);
+ mqttContext.setMaxPayloadSize(MAX_PAYLOAD_SIZE);
+ mqttContext.setUserName(USER_NAME);
+ mqttContext.setPassword(PASSWORD);
+ mqttContext.setLeakDetectorLevel(LEAK_DETECTOR_LEVEL);
}
@AfterEach
public void tearDown() {
- MqttContext context = new MqttContext();
- context.setPort(0);
- context.setBossGroupThreadCount(0);
- context.setWorkerGroupThreadCount(0);
- context.setMaxPayloadSize(0);
- context.setUserName(null);
- context.setPassword(null);
- context.setLeakDetectorLevel(null);
+ mqttContext.setPort(0);
+ mqttContext.setBossGroupThreadCount(0);
+ mqttContext.setWorkerGroupThreadCount(0);
+ mqttContext.setMaxPayloadSize(0);
+ mqttContext.setUserName(null);
+ mqttContext.setPassword(null);
+ mqttContext.setLeakDetectorLevel(null);
}
@Test
- public void settersShouldUpdateStaticState() {
- MqttContext context = new MqttContext();
-
- assertEquals(1883, context.getPort());
- assertEquals(1, context.getBossGroupThreadCount());
- assertEquals(2, context.getWorkerGroupThreadCount());
- assertEquals(1024, context.getMaxPayloadSize());
- assertEquals("test-user", context.getUserName());
- assertEquals("test-password", context.getPassword());
- assertEquals("disabled", context.getLeakDetectorLevel());
+ public void settersShouldBeReflectedByGetters() {
+ assertEquals(PORT, mqttContext.getPort());
+ assertEquals(BOSS_GROUP_THREAD_COUNT,
mqttContext.getBossGroupThreadCount());
+ assertEquals(WORKER_GROUP_THREAD_COUNT,
mqttContext.getWorkerGroupThreadCount());
+ assertEquals(MAX_PAYLOAD_SIZE, mqttContext.getMaxPayloadSize());
+ assertEquals(USER_NAME, mqttContext.getUserName());
+ assertEquals(PASSWORD, mqttContext.getPassword());
+ assertEquals(LEAK_DETECTOR_LEVEL, mqttContext.getLeakDetectorLevel());
}
@Test
- public void emptyUserNameOrPasswordShouldBeRejected() {
- byte[] password = "test-password".getBytes(StandardCharsets.UTF_8);
+ public void settingsShouldBeSharedBetweenInstances() {
+ MqttContext anotherContext = new MqttContext();
- assertFalse(MqttContext.isValid("", password));
- assertFalse(MqttContext.isValid("test-user", new byte[0]));
+ assertEquals(PORT, anotherContext.getPort());
+ assertEquals(USER_NAME, anotherContext.getUserName());
+ assertEquals(PASSWORD, anotherContext.getPassword());
}
- @Test
- public void mismatchedCredentialsShouldBeRejected() {
- byte[] password = "test-password".getBytes(StandardCharsets.UTF_8);
-
- assertFalse(MqttContext.isValid("another-user", password));
- assertFalse(MqttContext.isValid("test-user",
"another-password".getBytes(StandardCharsets.UTF_8)));
+ @ParameterizedTest(name = "userName=[{0}], password=[{1}] should be valid:
{2}")
+ @MethodSource("credentials")
+ public void isValidShouldCheckConfiguredCredentials(final String userName,
final byte[] passwordInBytes, final boolean expected) {
+ assertEquals(expected, MqttContext.isValid(userName, passwordInBytes));
}
- @Test
- public void validCredentialsShouldBeAccepted() {
- assertTrue(MqttContext.isValid("test-user",
"test-password".getBytes(StandardCharsets.UTF_8)));
+ private static Stream<Arguments> credentials() {
+ return Stream.of(
+ arguments(USER_NAME, PASSWORD_IN_BYTES, true),
+ arguments(USER_NAME, null, false),
+ arguments(USER_NAME, new byte[0], false),
+ arguments(null, PASSWORD_IN_BYTES, false),
+ arguments("", PASSWORD_IN_BYTES, false),
+ arguments(USER_NAME,
"wrong-password".getBytes(StandardCharsets.UTF_8), false),
+ arguments("wrong-user", PASSWORD_IN_BYTES, false),
+ arguments(USER_NAME.toUpperCase(), PASSWORD_IN_BYTES, false),
+ arguments(USER_NAME,
PASSWORD.toUpperCase().getBytes(StandardCharsets.UTF_8), false));
}
@Test
- public void nullUserNameArgumentShouldBeRejectedWithoutNpe() {
- assertFalse(MqttContext.isValid(null,
"test-password".getBytes(StandardCharsets.UTF_8)));
+ public void isValidShouldOnlyAcceptLatestConfiguredCredentials() {
+ String updatedUserName = "updated-user";
+ String updatedPassword = "updated-password";
+ mqttContext.setUserName(updatedUserName);
+ mqttContext.setPassword(updatedPassword);
+
+ assertTrue(MqttContext.isValid(updatedUserName,
updatedPassword.getBytes(StandardCharsets.UTF_8)));
+ assertFalse(MqttContext.isValid(USER_NAME, PASSWORD_IN_BYTES));
}
}
