From d1ab664a7683256681975c9f6871b53a3cd4a2ec Mon Sep 17 00:00:00 2001
From: Rainer Keller <Rainer.Keller@qt.io>
Date: Mon, 12 Nov 2018 14:57:20 +0100
Subject: Fix binary data encoding
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The encoding sends more bytes than expected because the node id was always
encoded as two bytes, even when it has to be skipped.
See Part6, Chapter 5.2.2.9,
Section "Two Byte NodeId Binary DataEncoding" and
Section "Four Byte NodeId Binary DataEncoding".

Change-Id: I004abffa057be3c4e58ea3cc81d0a3a679dd7103
Reviewed-by: Jannis Völker <jannis.voelker@basyskom.com>
Reviewed-by: Frank Meerkoetter <frank.meerkoetter@basyskom.com>
---
 src/opcua/client/qopcuabinarydataencoding.h | 37 +++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/src/opcua/client/qopcuabinarydataencoding.h b/src/opcua/client/qopcuabinarydataencoding.h
index 867015f..e22410c 100644
--- a/src/opcua/client/qopcuabinarydataencoding.h
+++ b/src/opcua/client/qopcuabinarydataencoding.h
@@ -368,7 +368,20 @@ inline QString QOpcUaBinaryDataEncoding::decode<QString, QOpcUa::Types::NodeId>(
 
     identifierType &= ~(0x40 | 0x80); // Remove expanded node id flags
 
-    quint16 namespaceIndex = decode<quint16>(success);
+    quint16 namespaceIndex;
+
+    if (identifierType == 0x00) {
+        // encodingType 0x00 does not transfer the namespace index, it has to be zero
+        // Part 6, Chapter 5.2.2.9, Section "Two Byte NodeId Binary DataEncoding"
+        namespaceIndex = 0;
+    } else if (identifierType == 0x01){
+        // encodingType 0x01 transfers only one byte namespace index, has to be in range 0-255
+        // Part 6, Chapter 5.2.2.9, Section "Four Byte NodeId Binary DataEncoding"
+        namespaceIndex = decode<quint8>(success);
+    } else {
+        namespaceIndex = decode<quint16>(success);
+    }
+
     if (!success)
         return QString();
 
@@ -739,12 +752,16 @@ inline bool QOpcUaBinaryDataEncoding::encode<QString, QOpcUa::Types::NodeId>(con
         if (!isNumber || integerIdentifier > upperBound<quint32>())
             return false;
 
-        if (integerIdentifier <= 255) {
+        if (integerIdentifier <= 255 && index == 0) {
+            // encodingType 0x00 does not transfer the namespace index, it has to be zero
+            // Part 6, Chapter 5.2.2.9, Section "Two Byte NodeId Binary DataEncoding"
             if (!encoder.encode<quint8>(integerIdentifier))
                 return false;
             encodingType = 0x00; // 8 bit numeric
             break;
-        } else if (integerIdentifier <= 65535) {
+        } else if (integerIdentifier <= 65535 && index <= 255) {
+            // encodingType 0x01 transfers only one byte namespace index, has to be in range 0-255
+            // Part 6, Chapter 5.2.2.9, Section "Four Byte NodeId Binary DataEncoding"
             if (!encoder.encode<quint16>(integerIdentifier))
                 return false;
             encodingType = 0x01; // 16 bit numeric
@@ -788,8 +805,18 @@ inline bool QOpcUaBinaryDataEncoding::encode<QString, QOpcUa::Types::NodeId>(con
 
     if (!encode<quint8>(encodingType))
         return false;
-    if (!encode<quint16>(index))
-        return false;
+
+    if (encodingType == 0x00) {
+        // encodingType == 0x00 skips namespace completely, defaults to zero
+        // Part 6, Chapter 5.2.2.9, Section "Two Byte NodeId Binary DataEncoding"
+    } else if (encodingType == 0x01) {
+        if (!encode<quint8>(index))
+            return false;
+    } else {
+        if (!encode<quint16>(index))
+            return false;
+    }
+
     m_data->append(encodedIdentifier);
     return true;
 }
-- 
cgit v1.2.3