fix(bacnetip): fix validation logic on NPDU header

apache · Sep 13, 2024 · 8d201d0 · 8d201d0
1 parent cf4f32d
commit 8d201d0
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 10 deletions.
diff --git a/plc4go/protocols/bacnetip/readwrite/model/NPDU.go b/plc4go/protocols/bacnetip/readwrite/model/NPDU.go
diff --git a/plc4j/drivers/bacnet/src/main/generated/org/apache/plc4x/java/bacnetip/readwrite/NPDU.java b/plc4j/drivers/bacnet/src/main/generated/org/apache/plc4x/java/bacnetip/readwrite/NPDU.java
@@ -300,10 +300,6 @@ public static NPDU staticParse(ReadBuffer readBuffer, Integer npduLength) throws
     NPDUControl control =
         readSimpleField(
             "control", readComplex(() -> NPDUControl.staticParse(readBuffer), readBuffer));
-    // Validation
-    if (!((control) != (null))) {
-      throw new ParseValidationException("control required for further processing");
-    }
 
     Integer destinationNetworkAddress =
         readOptionalField(
@@ -316,6 +312,13 @@ public static NPDU staticParse(ReadBuffer readBuffer, Integer npduLength) throws
             "destinationLength",
             readUnsignedShort(readBuffer, 8),
             control.getDestinationSpecified());
+    // Validation
+    if (!(((((control.getDestinationSpecified()) && ((destinationNetworkAddress) != (null)))
+            && ((destinationLength) != (null))))
+        || ((((!(control.getDestinationSpecified())) && ((destinationNetworkAddress) == (null)))
+            && ((destinationLength) == (null)))))) {
+      throw new ParseValidationException("inconsistent control");
+    }
 
     List<Short> destinationAddress =
         readCountArrayField(
@@ -335,6 +338,13 @@ public static NPDU staticParse(ReadBuffer readBuffer, Integer npduLength) throws
     Short sourceLength =
         readOptionalField(
             "sourceLength", readUnsignedShort(readBuffer, 8), control.getSourceSpecified());
+    // Validation
+    if (!(((((control.getSourceSpecified()) && ((sourceNetworkAddress) != (null)))
+            && ((sourceLength) != (null))))
+        || ((((!(control.getSourceSpecified())) && ((sourceNetworkAddress) == (null)))
+            && ((sourceLength) == (null)))))) {
+      throw new ParseValidationException("inconsistent control");
+    }
 
     List<Short> sourceAddress =
         readCountArrayField(

diff --git a/protocols/bacnetip/src/main/resources/protocols/bacnetip/bacnetip.mspec b/protocols/bacnetip/src/main/resources/protocols/bacnetip/bacnetip.mspec
@@ -98,14 +98,21 @@
 [type NPDU(uint 16 npduLength)
     [simple   uint 8      protocolVersionNumber                                                                   ]
     [simple   NPDUControl control                                                                                 ]
-    [validation    'control != null'    "control required for further processing"                                 ]
     [optional uint 16     destinationNetworkAddress   'control.destinationSpecified'                              ]
     [optional uint 8      destinationLength           'control.destinationSpecified'                              ]
+    // TODO: check if we don't treat a optional as optional when the condition meets
+    [validation    '(control.destinationSpecified && destinationNetworkAddress != null && destinationLength != null)
+                 || (!control.destinationSpecified && destinationNetworkAddress == null && destinationLength == null)'
+                    "inconsistent control"                                                                        ]
     [array    uint 8      destinationAddress count    'control.destinationSpecified ? destinationLength : 0'      ]
                                                         // (destinationNetworkAddress(16bit) + destinationLength(8bit) + destinationLength)?
     [virtual  uint 16     destinationLengthAddon      'control.destinationSpecified ? (3 + destinationLength) : 0'  ]
     [optional uint 16     sourceNetworkAddress        'control.sourceSpecified'                                   ]
     [optional uint 8      sourceLength                'control.sourceSpecified'                                   ]
+    // TODO: check if we don't treat a optional as optional when the condition meets
+    [validation    '(control.sourceSpecified && sourceNetworkAddress != null && sourceLength != null)
+                 || (!control.sourceSpecified && sourceNetworkAddress == null && sourceLength == null)'
+                    "inconsistent control"                                                                        ]
     [array    uint 8      sourceAddress count         'control.sourceSpecified ? sourceLength : 0'                ]
                                                         // (sourceNetworkAddress(16bit) + sourceLength(8bit) + sourceLength)?
     [virtual  uint 16     sourceLengthAddon           'control.sourceSpecified ? (3 + sourceLength) : 0'            ]