Corrected bug in all confirmed handlers: if a segmented message was received, the handler tried to decode it instead of immediately sending an abort. This caused lockup with WriteProperty.

bacnet-stack/ports/pic18f6720/h_rp.c

@@ -61,22 +61,25 @@ void handler_read_property(uint8_t * service_request,
     BACNET_ERROR_CODE error_code = ERROR_CODE_UNKNOWN_OBJECT;
     BACNET_ADDRESS my_address;
 
-    len = rp_decode_service_request(service_request, service_len, &data);
     /* encode the NPDU portion of the packet */
     datalink_get_my_address(&my_address);
     npdu_encode_npdu_data(&npdu_data, false, MESSAGE_PRIORITY_NORMAL);
     pdu_len = npdu_encode_pdu(&Handler_Transmit_Buffer[0], src,
         &my_address, &npdu_data);
-    if (len < 0) {
+    if (service_data->segmented_message) {
-        /* bad decoding - send an abort */
-        len = abort_encode_apdu(&Handler_Transmit_Buffer[pdu_len],
-            service_data->invoke_id, ABORT_REASON_OTHER, true);
-    } else if (service_data->segmented_message) {
         /* we don't support segmentation - send an abort */
         len = abort_encode_apdu(&Handler_Transmit_Buffer[pdu_len],
             service_data->invoke_id,
             ABORT_REASON_SEGMENTATION_NOT_SUPPORTED, true);
-    } else {
+        goto RP_ABORT;
+    }
+    len = rp_decode_service_request(service_request, service_len, &data);
+    if (len < 0) {
+        /* bad decoding - send an abort */
+        len = abort_encode_apdu(&Handler_Transmit_Buffer[pdu_len],
+            service_data->invoke_id, ABORT_REASON_OTHER, true);
+        goto RP_ABORT;
+    }
     /* most cases will be error */
     error = true;
     switch (data.object_type) {
@@ -173,23 +176,19 @@ void handler_read_property(uint8_t * service_request,
     default:
         break;
     }
-    }
     if (error) {
-        switch (len) {
+        if (len == -2) {
             /* BACnet APDU too small to fit data, so proper response is Abort */
-        case -2:
             len = abort_encode_apdu(&Handler_Transmit_Buffer[pdu_len],
                 service_data->invoke_id,
                 ABORT_REASON_SEGMENTATION_NOT_SUPPORTED, true);
-            break;
+            goto RP_ABORT;
-        case -1:
+        }
-        default:
         len = bacerror_encode_apdu(&Handler_Transmit_Buffer[pdu_len],
             service_data->invoke_id,
             SERVICE_CONFIRMED_READ_PROPERTY, error_class, error_code);
-            break;
-        }
     }
+RP_ABORT:
     pdu_len += len;
     bytes_sent = datalink_send_pdu(src, &npdu_data,
         &Handler_Transmit_Buffer[0], pdu_len);

bacnet-stack/ports/pic18f6720/h_wp.c

@@ -57,39 +57,26 @@ void handler_write_property(uint8_t * service_request,
     int bytes_sent = 0;
     BACNET_ADDRESS my_address;
 
-    /* decode the service request only */
-    len = wp_decode_service_request(service_request,
-        service_len, &wp_data);
     /* encode the NPDU portion of the packet */
     datalink_get_my_address(&my_address);
     npdu_encode_npdu_data(&npdu_data, false, MESSAGE_PRIORITY_NORMAL);
     pdu_len = npdu_encode_pdu(&Handler_Transmit_Buffer[0], src,
         &my_address, &npdu_data);
-#if PRINT_ENABLED
+    if (service_data->segmented_message) {
-    fprintf(stderr, "Received Write-Property Request!\n");
+        len = abort_encode_apdu(&Handler_Transmit_Buffer[pdu_len],
-    if (len > 0)
+            service_data->invoke_id,
-        fprintf(stderr, "type=%u instance=%u property=%u index=%d\n",
+            ABORT_REASON_SEGMENTATION_NOT_SUPPORTED, true);
-            wp_data.object_type,
+        goto WP_ABORT;
-            wp_data.object_instance,
+    }
-            wp_data.object_property, wp_data.array_index);
+    /* decode the service request only */
-    else
+    len = wp_decode_service_request(service_request,
-        fprintf(stderr, "Unable to decode Write-Property Request!\n");
+        service_len, &wp_data);
-#endif
     /* bad decoding or something we didn't understand - send an abort */
     if (len <= 0) {
         len = abort_encode_apdu(&Handler_Transmit_Buffer[pdu_len],
             service_data->invoke_id, ABORT_REASON_OTHER, true);
-#if PRINT_ENABLED
+        goto WP_ABORT;
-        fprintf(stderr, "Sending Abort!\n");
+    } 
-#endif
-    } else if (service_data->segmented_message) {
-        len = abort_encode_apdu(&Handler_Transmit_Buffer[pdu_len],
-            service_data->invoke_id,
-            ABORT_REASON_SEGMENTATION_NOT_SUPPORTED, true);
-#if PRINT_ENABLED
-        fprintf(stderr, "Sending Abort!\n");
-#endif
-    } else {
     switch (wp_data.object_type) {
     case OBJECT_DEVICE:
         if (Device_Write_Property(&wp_data, &error_class, &error_code)) {
@@ -179,7 +166,7 @@ void handler_write_property(uint8_t * service_request,
 #endif
         break;
     }
-    }
+WP_ABORT:
     pdu_len += len;
     bytes_sent = datalink_send_pdu(src, &npdu_data,
         &Handler_Transmit_Buffer[0], pdu_len);