Skip to content

<fix>[l2]: null-safe type check in ChangeL2NetworkVlanId #3722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zstack-robot-2 wants to merge 1 commit into from
+5 −5
Closed

<fix>[l2]: null-safe type check in ChangeL2NetworkVlanId #3722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions network/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private void validate(APIChangeL2NetworkVlanIdMsg msg) {
}
String sdnControllerUuid = L2NetworkSystemTags.L2_NETWORK_SDN_CONTROLLER_UUID
.getTokenByResourceUuid(msg.getL2NetworkUuid(), L2NetworkSystemTags.L2_NETWORK_SDN_CONTROLLER_UUID_TOKEN);
if (msg.getType().equals(L2NetworkConstant.L2_VLAN_NETWORK_TYPE)) {
if (L2NetworkConstant.L2_VLAN_NETWORK_TYPE.equals(msg.getType())) {
if (msg.getVlan() == null) {
throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_10016, "vlan is required for " +
"ChangeL2NetworkVlanId with type[%s]", msg.getType()));
Expand Down Expand Up @@ -190,7 +190,7 @@ private void validate(APIChangeL2NetworkVlanIdMsg msg) {
throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_10018, "There has been a l2Network attached to cluster with virtual network id[%s] and physical interface[%s]. Failed to change L2 network[uuid:%s]",
msg.getVlan(), l2.getPhysicalInterface(), l2.getUuid()));
}
} else if (msg.getType().equals(L2NetworkConstant.L2_NO_VLAN_NETWORK_TYPE)) {
} else if (L2NetworkConstant.L2_NO_VLAN_NETWORK_TYPE.equals(msg.getType())) {
if (msg.getVlan() != null) {
throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_10019, "vlan is not allowed for " +
"ChangeL2NetworkVlanId with type[%s]", msg.getType()));
Expand Down
4 changes: 2 additions & 2 deletions network/src/main/java/org/zstack/network/l2/L2NoVlanNetwork.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,14 +422,14 @@ private void handleApiMessage(APIMessage msg) {
private void handle(APIChangeL2NetworkVlanIdMsg msg){
APIChangeL2NetworkVlanIdEvent event = new APIChangeL2NetworkVlanIdEvent(msg.getId());
if (msg.getVlan() == null
&& msg.getType().equals(L2NetworkConstant.L2_NO_VLAN_NETWORK_TYPE)
&& L2NetworkConstant.L2_NO_VLAN_NETWORK_TYPE.equals(msg.getType())
&& self.getVirtualNetworkId().equals(0)) {
event.setInventory(getSelfInventory());
bus.publish(event);
return;
}
if (msg.getVlan() != null
&& msg.getType().equals(L2NetworkConstant.L2_VLAN_NETWORK_TYPE)
&& L2NetworkConstant.L2_VLAN_NETWORK_TYPE.equals(msg.getType())
&& self.getVirtualNetworkId().equals(msg.getVlan())) {
Comment on lines +425 to 433
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

type 为空时会落入后续写库路径，存在写入非法类型风险

Line 425-433 现在不会再因空值抛 NPE，但空 type 会继续执行到后续链路；同文件 Line 545-553 会把 msg.getType() 作为 targetType 写回 L2NetworkVO.type。当 type == null 时，这里会产生错误状态或数据库异常。

💡 建议修复 
 private void handle(APIChangeL2NetworkVlanIdMsg msg){
     APIChangeL2NetworkVlanIdEvent event = new APIChangeL2NetworkVlanIdEvent(msg.getId());
+    String effectiveType = msg.getType() == null ? self.getType() : msg.getType();
+    if (msg.getType() == null) {
+        msg.setType(effectiveType);
+    }
+
     if (msg.getVlan() == null
-            && L2NetworkConstant.L2_NO_VLAN_NETWORK_TYPE.equals(msg.getType())
+            && L2NetworkConstant.L2_NO_VLAN_NETWORK_TYPE.equals(effectiveType)
             && self.getVirtualNetworkId().equals(0)) {
         event.setInventory(getSelfInventory());
         bus.publish(event);
         return;
     }
     if (msg.getVlan() != null
-            && L2NetworkConstant.L2_VLAN_NETWORK_TYPE.equals(msg.getType())
+            && L2NetworkConstant.L2_VLAN_NETWORK_TYPE.equals(effectiveType)
             && self.getVirtualNetworkId().equals(msg.getVlan())) {
         event.setInventory(getSelfInventory());
         bus.publish(event);
         return;
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@network/src/main/java/org/zstack/network/l2/L2NoVlanNetwork.java` around
lines 425 - 433, The null/empty msg.getType() can flow past the early-return in
L2NoVlanNetwork and later be persisted into L2NetworkVO.type, causing invalid DB
state; update the message handling in L2NoVlanNetwork (where the code checks
L2NetworkConstant.L2_NO_VLAN_NETWORK_TYPE and
L2NetworkConstant.L2_VLAN_NETWORK_TYPE) to explicitly guard msg.getType() for
null/empty (e.g., return/error) before any further logic, and ensure the code
path that writes back to L2NetworkVO.type (the block that uses msg.getType() as
targetType) only uses a non-null, validated type or substitutes a safe
default/throws a controlled error.

event.setInventory(getSelfInventory());
bus.publish(event);
Expand Down
2 changes: 1 addition & 1 deletion ...n/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanNetworkCheckerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public APIMessage intercept(APIMessage msg) throws ApiMessageInterceptionExcepti
}

private void validate(APIChangeL2NetworkVlanIdMsg msg) {
if (!msg.getType().equals(VxlanNetworkConstant.VXLAN_NETWORK_TYPE)){
if (!VxlanNetworkConstant.VXLAN_NETWORK_TYPE.equals(msg.getType())){
return;
}
Comment on lines +41 to 43
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

空 type 会直接绕过 VXLAN 的 VNI 校验路径

Line 41 在 type == null 时会直接 return，导致后续 Line 44-60 的 VNI 合法性/重复性检查被跳过。type 是可选字段，这里建议先推导 effectiveType（为空时按当前 L2 实际类型判断），再决定是否进入 VXLAN 校验。

💡 建议修复 
 private void validate(APIChangeL2NetworkVlanIdMsg msg) {
-    if (!VxlanNetworkConstant.VXLAN_NETWORK_TYPE.equals(msg.getType())){
+    String effectiveType = msg.getType();
+    if (effectiveType == null) {
+        effectiveType = Q.New(L2NetworkVO.class)
+                .select(L2NetworkVO_.type)
+                .eq(L2NetworkVO_.uuid, msg.getL2NetworkUuid())
+                .findValue();
+    }
+    if (!VxlanNetworkConstant.VXLAN_NETWORK_TYPE.equals(effectiveType)) {
         return;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!VxlanNetworkConstant.VXLAN_NETWORK_TYPE.equals(msg.getType())){
return;
}
String effectiveType = msg.getType();
if (effectiveType == null) {
effectiveType = Q.New(L2NetworkVO.class)
.select(L2NetworkVO_.type)
.eq(L2NetworkVO_.uuid, msg.getL2NetworkUuid())
.findValue();
}
if (!VxlanNetworkConstant.VXLAN_NETWORK_TYPE.equals(effectiveType)) {
return;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanNetworkCheckerImpl.java`
around lines 41 - 43, The check in VxlanNetworkCheckerImpl currently returns
when msg.getType() is null, skipping VNI validation; instead compute an
effectiveType (use msg.getType() if non-null, otherwise load the L2 network by
msg.getL2NetworkUuid() and read its type) and compare that effectiveType to
VxlanNetworkConstant.VXLAN_NETWORK_TYPE before running the VNI
validity/duplication checks present in the VNI check block (the logic around the
current 44-60 region). Ensure the code uses the effectiveType variable in the
comparison and only returns early for non-VXLAN effective types so null type
messages still trigger VXLAN VNI checks.

if (!NetworkUtils.isValidVni(msg.getVlan())) {
Expand Down