You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by cd...@apache.org on 2019/10/16 10:51:15 UTC

[plc4x] 01/01: PLC4X-144 - When requesting invalid addresses, the DefaultS7MessageProcessor produces errors - The DefaultS7MessageProcessor didn't inspect the return code and tried to merge invalid fields, hereby causing array index errors ... by checking the status code this should be fixed

This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a commit to branch feature/PLC4X-144
in repository https://gitbox.apache.org/repos/asf/plc4x.git

commit ebb1bad05938592207914ce53c417be3a8bf5379
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Wed Oct 16 12:51:07 2019 +0200

    PLC4X-144 - When requesting invalid addresses, the DefaultS7MessageProcessor produces errors
    - The DefaultS7MessageProcessor didn't inspect the return code and tried to merge invalid fields, hereby causing array index errors ... by checking the status code this should be fixed
---
 .../strategies/DefaultS7MessageProcessor.java      | 51 ++++++++++++----------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java
index cbb3243..4e83e14 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java
@@ -396,37 +396,42 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor {
                 // Get the pairs of corresponding parameter and payload items.
                 S7AnyVarParameterItem responseParameterItem = (S7AnyVarParameterItem) parameterItems.get(0);
                 VarPayloadItem responsePayloadItem = payloadItems.get(i + responseOffset);
-                int dataOffset = (responsePayloadItem.getData() != null) ? responsePayloadItem.getData().length : 0;
 
-                // The resulting parameter items is identical to the request parameter item.
-                mergedParameterItems.add(requestItem);
+                if(responsePayloadItem.getReturnCode() == DataTransportErrorCode.OK) {
+                    int dataOffset = (responsePayloadItem.getData() != null) ? responsePayloadItem.getData().length : 0;
 
-                // The payload will have to be merged and the return codes will have to be examined.
-                if(requestItem.getNumElements() != responseParameterItem.getNumElements()) {
-                    int itemSizeInBytes = requestItem.getDataType().getSizeInBytes();
-                    int totalSizeInBytes = requestItem.getNumElements() * itemSizeInBytes;
+                    // The resulting parameter items is identical to the request parameter item.
+                    mergedParameterItems.add(requestItem);
 
-                    if(varParameter.getType() == ParameterType.READ_VAR) {
-                        byte[] data = new byte[totalSizeInBytes];
-                        System.arraycopy(responsePayloadItem.getData(), 0, data, 0, responsePayloadItem.getData().length);
+                    // The payload will have to be merged and the return codes will have to be examined.
+                    if (requestItem.getNumElements() != responseParameterItem.getNumElements()) {
+                        int itemSizeInBytes = requestItem.getDataType().getSizeInBytes();
+                        int totalSizeInBytes = requestItem.getNumElements() * itemSizeInBytes;
 
-                        // Now iterate over the succeeding pairs of parameters and payloads till we have
-                        // found the original number of elements.
-                        while (dataOffset < totalSizeInBytes) {
-                            responseOffset++;
+                        if (varParameter.getType() == ParameterType.READ_VAR) {
+                            byte[] data = new byte[totalSizeInBytes];
+                            System.arraycopy(responsePayloadItem.getData(), 0, data, 0, responsePayloadItem.getData().length);
 
-                            // Get the next payload item in the list.
-                            responsePayloadItem = payloadItems.get(i + responseOffset);
+                            // Now iterate over the succeeding pairs of parameters and payloads till we have
+                            // found the original number of elements.
+                            while (dataOffset < totalSizeInBytes) {
+                                responseOffset++;
 
-                            // Copy the data of this item behind the previous content.
-                            if (varParameter.getType() == ParameterType.READ_VAR) {
-                                System.arraycopy(responsePayloadItem.getData(), 0, data, dataOffset, responsePayloadItem.getData().length);
-                                dataOffset += responsePayloadItem.getData().length;
+                                // Get the next payload item in the list.
+                                responsePayloadItem = payloadItems.get(i + responseOffset);
+
+                                // Copy the data of this item behind the previous content.
+                                if (varParameter.getType() == ParameterType.READ_VAR) {
+                                    System.arraycopy(responsePayloadItem.getData(), 0, data, dataOffset, responsePayloadItem.getData().length);
+                                    dataOffset += responsePayloadItem.getData().length;
+                                }
                             }
-                        }
 
-                        mergedPayloadItems.add(new VarPayloadItem(DataTransportErrorCode.OK,
-                            responsePayloadItem.getDataTransportSize(), data));
+                            mergedPayloadItems.add(new VarPayloadItem(DataTransportErrorCode.OK,
+                                responsePayloadItem.getDataTransportSize(), data));
+                        }
+                    } else {
+                        mergedPayloadItems.add(responsePayloadItem);
                     }
                 } else {
                     mergedPayloadItems.add(responsePayloadItem);