This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new e3dee797e fix(instance): bound instance text to the column width and
read a legacy null vendor (#4713)
e3dee797e is described below
commit e3dee797ef003903134c342aa6be89bfd7143621
Author: btlqql <[email protected]>
AuthorDate: Mon Sep 21 20:16:09 2026 +0800
fix(instance): bound instance text to the column width and read a legacy
null vendor (#4713)
Two instance-metadata robustness fixes from the same author, consolidated
into one change.
1. `InstanceService` accepted `endpoint` / `remark` / `adminCredentialRef`
of any length and let the database reject them; they are now validated against
the widths declared in `db/schema.sql` (255 / 512 / 128) and fail with
`BusinessException(400)`.
2. `MybatisPlusInstanceRepository.parseVendor` turned a null or blank
`vendor` column into a 500. Legacy rows written before the column existed now
read as `APACHE`; genuinely unknown values still fail loudly.
Consolidates #4713 and #4716 (same author, same domain). Line-disjoint from
#4692, which was returned to its author separately.
---
.../rocketmq/studio/instance/InstanceService.java | 30 +++++-
.../instance/MybatisPlusInstanceRepository.java | 9 +-
.../studio/instance/InstanceServiceTest.java | 106 +++++++++++++++++++++
.../MybatisPlusInstanceRepositoryTest.java | 26 +++++
4 files changed, 166 insertions(+), 5 deletions(-)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/instance/InstanceService.java
b/server/src/main/java/org/apache/rocketmq/studio/instance/InstanceService.java
index e916d35dd..62a41fdbd 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/instance/InstanceService.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/instance/InstanceService.java
@@ -219,6 +219,7 @@ public class InstanceService {
case ALIYUN, TENCENT -> createCloudInstance(instance, vendor);
}
+ instance.setRemark(requireTextWithin(instance.getRemark(),
MAX_INSTANCE_REMARK_LENGTH, "remark"));
requireUniqueInstanceName(instance.getName(), null);
instance.setGmtCreate(LocalDateTime.now());
instance.setGmtModified(LocalDateTime.now());
@@ -467,7 +468,8 @@ public class InstanceService {
instance.setVendor(InstanceVendor.APACHE);
instance.setName(requireInstanceName(instance.getName()));
instance.setEndpoint(requireValidEndpoint(instance.getEndpoint()));
-
instance.setAdminCredentialRef(normalizeCredentialRef(instance.getAdminCredentialRef()));
+
instance.setAdminCredentialRef(requireTextWithin(normalizeCredentialRef(instance.getAdminCredentialRef()),
+ MAX_INSTANCE_CREDENTIAL_REF_LENGTH, "adminCredentialRef"));
if (instance.getType() == null) {
throw new BusinessException(400, "InstanceVO type is required");
}
@@ -542,7 +544,7 @@ public class InstanceService {
if (!StringUtils.hasText(endpoint)) {
throw new BusinessException(400, "InstanceVO endpoint is
required");
}
- String normalized = endpoint.trim();
+ String normalized = requireTextWithin(endpoint.trim(),
MAX_INSTANCE_ENDPOINT_LENGTH, "endpoint");
for (String address : normalized.split("[;,]", -1)) {
if (address.isBlank()) {
throw new BusinessException(400, "InstanceVO endpoint must not
contain empty addresses");
@@ -562,6 +564,24 @@ public class InstanceService {
return trimmed;
}
+ /** Free-text fields of rmq_instance, capped at the width of their column.
*/
+ static final int MAX_INSTANCE_ENDPOINT_LENGTH = 512;
+ static final int MAX_INSTANCE_REMARK_LENGTH = 255;
+ static final int MAX_INSTANCE_CREDENTIAL_REF_LENGTH = 128;
+
+ /**
+ * Bounds a free-text field to the width of its rmq_instance column.
Letting a longer value
+ * through does not store it: MySQL rejects the write, so the caller gets
a 500 from the
+ * persistence layer instead of the validation error the name field
already returns.
+ */
+ private static String requireTextWithin(String value, int maxLength,
String field) {
+ if (value != null && value.length() > maxLength) {
+ throw new BusinessException(400, "InstanceVO " + field + " must
not exceed "
+ + maxLength + " characters");
+ }
+ return value;
+ }
+
private String normalizeCredentialRef(String credentialRef) {
return StringUtils.hasText(credentialRef) ? credentialRef.trim() :
null;
}
@@ -601,10 +621,12 @@ public class InstanceService {
}
}
if (instance.getRemark() != null) {
- updated.setRemark(instance.getRemark());
+ updated.setRemark(requireTextWithin(instance.getRemark(),
MAX_INSTANCE_REMARK_LENGTH, "remark"));
}
if (!cloudInstance && instance.getAdminCredentialRef() != null) {
-
updated.setAdminCredentialRef(normalizeCredentialRef(instance.getAdminCredentialRef()));
+ updated.setAdminCredentialRef(requireTextWithin(
+ normalizeCredentialRef(instance.getAdminCredentialRef()),
+ MAX_INSTANCE_CREDENTIAL_REF_LENGTH, "adminCredentialRef"));
}
updated.setGmtModified(LocalDateTime.now());
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepository.java
b/server/src/main/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepository.java
index f63f9b4e7..7fcd1a33f 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepository.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepository.java
@@ -192,9 +192,16 @@ public class MybatisPlusInstanceRepository implements
InstanceRepository {
}
private InstanceVendor parseVendor(Long instanceId, String vendor) {
+ if (vendor == null || vendor.isBlank()) {
+ // vendor is an optional column and every writer stores a name, so
a missing value is
+ // a row written before the column existed. Every reader treats
that as APACHE
+ // (InstanceService, InstanceCapabilityService, the metrics
collectors), so only a
+ // value that names no vendor at all is a corrupt row.
+ return InstanceVendor.APACHE;
+ }
try {
return InstanceVendor.valueOf(vendor);
- } catch (IllegalArgumentException | NullPointerException ex) {
+ } catch (IllegalArgumentException ex) {
throw invalidPersistedValue(instanceId, "vendor", vendor);
}
}
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/instance/InstanceServiceTest.java
b/server/src/test/java/org/apache/rocketmq/studio/instance/InstanceServiceTest.java
index 4d41e3911..d1b21a0d6 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/instance/InstanceServiceTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/instance/InstanceServiceTest.java
@@ -511,6 +511,112 @@ class InstanceServiceTest {
.hasMessage("InstanceVO endpoint must not contain empty
addresses");
}
+ @Test
+ void createInstanceShouldBoundTheEndpointToTheColumnWidthTest() {
+
when(instanceRepository.save(any(InstanceVO.class))).thenAnswer(invocation ->
invocation.getArgument(0));
+ String atLimit = "n".repeat(512);
+ InstanceVO accepted =
InstanceVO.builder().name("inst-a").type(InstanceType.PROXY_CLUSTER)
+ .endpoint(atLimit).build();
+
+
assertThat(instanceService.createInstance(accepted).getEndpoint()).isEqualTo(atLimit);
+
+ InstanceVO rejected =
InstanceVO.builder().name("inst-b").type(InstanceType.PROXY_CLUSTER)
+ .endpoint(atLimit + ";namesrv-2:9876").build();
+ assertThatThrownBy(() -> instanceService.createInstance(rejected))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("InstanceVO endpoint must not exceed 512
characters");
+ verify(instanceRepository, never()).save(argThat(instance ->
instance.getEndpoint().length() > 512));
+ }
+
+ @Test
+ void updateInstanceShouldBoundTheEndpointToTheColumnWidthTest() {
+ InstanceVO existing =
InstanceVO.builder().name("inst-a").type(InstanceType.PROXY_CLUSTER)
+ .endpoint("namesrv:9876").build();
+ existing.setId(1L);
+ String atLimit = "n".repeat(512);
+ InstanceVO accepted = InstanceVO.builder().endpoint(atLimit).build();
+ accepted.setId(1L);
+ InstanceVO rejected = InstanceVO.builder().endpoint(atLimit +
"n").build();
+ rejected.setId(1L);
+
when(instanceRepository.findById(1L)).thenReturn(Optional.of(existing));
+
when(instanceRepository.save(any(InstanceVO.class))).thenAnswer(invocation ->
invocation.getArgument(0));
+
+
assertThat(instanceService.updateInstance(accepted).getEndpoint()).isEqualTo(atLimit);
+ assertThatThrownBy(() -> instanceService.updateInstance(rejected))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("InstanceVO endpoint must not exceed 512
characters");
+ }
+
+ @Test
+ void createInstanceShouldBoundTheRemarkToTheColumnWidthTest() {
+
when(instanceRepository.save(any(InstanceVO.class))).thenAnswer(invocation ->
invocation.getArgument(0));
+ String atLimit = "r".repeat(255);
+ InstanceVO accepted =
InstanceVO.builder().name("inst-a").type(InstanceType.PROXY_CLUSTER)
+ .endpoint("namesrv:9876").remark(atLimit).build();
+
+
assertThat(instanceService.createInstance(accepted).getRemark()).isEqualTo(atLimit);
+
+ InstanceVO rejected =
InstanceVO.builder().name("inst-b").type(InstanceType.PROXY_CLUSTER)
+ .endpoint("namesrv:9876").remark(atLimit + "r").build();
+ assertThatThrownBy(() -> instanceService.createInstance(rejected))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("InstanceVO remark must not exceed 255
characters");
+ verify(instanceRepository, never()).save(argThat(instance ->
instance.getRemark() != null
+ && instance.getRemark().length() > 255));
+ }
+
+ @Test
+ void updateInstanceShouldBoundTheRemarkToTheColumnWidthTest() {
+ InstanceVO existing =
InstanceVO.builder().name("inst-a").type(InstanceType.PROXY_CLUSTER)
+ .endpoint("namesrv:9876").build();
+ existing.setId(1L);
+ String atLimit = "r".repeat(255);
+ InstanceVO accepted = InstanceVO.builder().remark(atLimit).build();
+ accepted.setId(1L);
+ InstanceVO rejected = InstanceVO.builder().remark(atLimit +
"r").build();
+ rejected.setId(1L);
+
when(instanceRepository.findById(1L)).thenReturn(Optional.of(existing));
+
when(instanceRepository.save(any(InstanceVO.class))).thenAnswer(invocation ->
invocation.getArgument(0));
+
+
assertThat(instanceService.updateInstance(accepted).getRemark()).isEqualTo(atLimit);
+ assertThatThrownBy(() -> instanceService.updateInstance(rejected))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("InstanceVO remark must not exceed 255
characters");
+ }
+ @Test
+ void
createInstanceShouldBoundTheAdminCredentialReferenceToTheColumnWidthTest() {
+
when(instanceRepository.save(any(InstanceVO.class))).thenAnswer(invocation ->
invocation.getArgument(0));
+ String atLimit = "c".repeat(128);
+ InstanceVO accepted =
InstanceVO.builder().name("inst-a").type(InstanceType.PROXY_CLUSTER)
+ .endpoint("namesrv:9876").adminCredentialRef(atLimit).build();
+
+
assertThat(instanceService.createInstance(accepted).getAdminCredentialRef()).isEqualTo(atLimit);
+
+ InstanceVO rejected =
InstanceVO.builder().name("inst-b").type(InstanceType.PROXY_CLUSTER)
+ .endpoint("namesrv:9876").adminCredentialRef(atLimit +
"c").build();
+ assertThatThrownBy(() -> instanceService.createInstance(rejected))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("InstanceVO adminCredentialRef must not exceed 128
characters");
+ }
+
+ @Test
+ void
updateInstanceShouldBoundTheAdminCredentialReferenceToTheColumnWidthTest() {
+ InstanceVO existing =
InstanceVO.builder().name("inst-a").type(InstanceType.PROXY_CLUSTER)
+ .endpoint("namesrv:9876").build();
+ existing.setId(1L);
+ String atLimit = "c".repeat(128);
+ InstanceVO accepted =
InstanceVO.builder().adminCredentialRef(atLimit).build();
+ accepted.setId(1L);
+ InstanceVO rejected = InstanceVO.builder().adminCredentialRef(atLimit
+ "c").build();
+ rejected.setId(1L);
+
when(instanceRepository.findById(1L)).thenReturn(Optional.of(existing));
+
when(instanceRepository.save(any(InstanceVO.class))).thenAnswer(invocation ->
invocation.getArgument(0));
+
+
assertThat(instanceService.updateInstance(accepted).getAdminCredentialRef()).isEqualTo(atLimit);
+ assertThatThrownBy(() -> instanceService.updateInstance(rejected))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("InstanceVO adminCredentialRef must not exceed 128
characters");
+ }
@Test
void createInstanceShouldTrimEndpointBeforeSaving() {
InstanceVO input =
InstanceVO.builder().name("valid-name").type(InstanceType.PROXY_CLUSTER)
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepositoryTest.java
b/server/src/test/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepositoryTest.java
index c01d64b74..d5f21dba2 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepositoryTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/instance/MybatisPlusInstanceRepositoryTest.java
@@ -170,6 +170,32 @@ class MybatisPlusInstanceRepositoryTest {
.hasMessageContaining(String.valueOf(entity.getId()));
}
+ @Test
+ void findAllShouldDefaultALegacyNullVendorToApacheTest() {
+ // vendor is a nullable column: a row written before the column
existed carries no
+ // vendor, and every reader in the codebase treats that as APACHE.
+ RmqInstance legacy = entity(5L, "instance-legacy",
InstanceType.DIRECT);
+ legacy.setVendor(null);
+
when(instanceMapper.selectList(any(QueryWrapper.class))).thenReturn(List.of(legacy));
+
+ List<InstanceVO> result = repository.findAll();
+
+ assertThat(result).singleElement()
+ .satisfies(instance ->
assertThat(instance.getVendor()).isEqualTo(InstanceVendor.APACHE));
+ }
+
+ @Test
+ void findByIdShouldDefaultABlankVendorToApacheTest() {
+ RmqInstance legacy = entity(6L, "instance-legacy-blank",
InstanceType.PROXY_LOCAL);
+ legacy.setVendor("");
+ when(instanceMapper.selectById(6L)).thenReturn(legacy);
+
+ Optional<InstanceVO> result = repository.findById(6L);
+
+ assertThat(result).isPresent();
+ assertThat(result.get().getVendor()).isEqualTo(InstanceVendor.APACHE);
+ }
+
@Test
void countTopicsByInstanceShouldDelegateToTopicMapperTest() {
when(topicMapper.selectCount(any(QueryWrapper.class))).thenReturn(5L);
