fix #541

src/server/ua_server_binary.c

@@ -502,11 +502,9 @@ processRequest(UA_SecureChannel *channel, UA_Server *server, UA_UInt32 requestId
     /* Send the response */
     init_response_header(request, response);
     retval = UA_SecureChannel_sendBinaryMessage(channel, requestId, response, responseType);
-    if(retval != UA_STATUSCODE_GOOD) {
-        UA_LOG_DEBUG_CHANNEL(server->config.logger, channel, "Could not send the message over "
+    if(retval != UA_STATUSCODE_GOOD)
+        UA_LOG_INFO_CHANNEL(server->config.logger, channel, "Could not send the message over "
                              "the SecureChannel with error code 0x%08x", retval);
-        sendError(channel, msg, requestPos, &UA_TYPES[UA_TYPES_SERVICEFAULT], requestId, retval);
-    }
 
     /* Clean up */
     UA_deleteMembers(request, requestType);

src/ua_securechannel.c

@@ -4,7 +4,6 @@
 #include "ua_types_encoding_binary.h"
 #include "ua_types_generated_encoding_binary.h"
 #include "ua_transport_generated_encoding_binary.h"
-#include <stdio.h>
 
 #define UA_SECURE_MESSAGE_HEADER_LENGTH 24
 
@@ -140,35 +139,35 @@ UA_SecureChannel_sendChunk(UA_ChunkInfo *ci, UA_ByteString *dst, size_t offset)
     dst->length += UA_SECURE_MESSAGE_HEADER_LENGTH;
     offset += UA_SECURE_MESSAGE_HEADER_LENGTH;
 
-    UA_Boolean abortMsg = (ci->abort || ++ci->chunksSoFar > connection->remoteConf.maxChunkCount ||
-                           ci->messageSizeSoFar + offset > connection->remoteConf.maxMessageSize);
+    if(++ci->chunksSoFar > connection->remoteConf.maxChunkCount ||
+       ci->messageSizeSoFar + offset > connection->remoteConf.maxMessageSize)
+        ci->errorCode = UA_STATUSCODE_BADTCPMESSAGETOOLARGE;
 
     /* Prepare the chunk headers */
     UA_SecureConversationMessageHeader respHeader;
     respHeader.secureChannelId = channel->securityToken.channelId;
     respHeader.messageHeader.messageTypeAndChunkType = ci->messageType;
-    if(!abortMsg) {
+    if(ci->errorCode == UA_STATUSCODE_GOOD) {
         if(ci->final)
             respHeader.messageHeader.messageTypeAndChunkType += UA_CHUNKTYPE_FINAL;
         else
             respHeader.messageHeader.messageTypeAndChunkType += UA_CHUNKTYPE_INTERMEDIATE;
     } else {
+        /* abort message */
+        ci->final = true; /* mark as finished */
         respHeader.messageHeader.messageTypeAndChunkType += UA_CHUNKTYPE_ABORT;
-        ci->abort = true;
-        UA_StatusCode retval = UA_STATUSCODE_BADTCPMESSAGETOOLARGE;
-        UA_String errorMsg = UA_STRING("Encoded message too long");
+        UA_String errorMsg;
+        UA_String_init(&errorMsg);
         offset = UA_SECURE_MESSAGE_HEADER_LENGTH;
-        UA_UInt32_encodeBinary(&retval,dst,&offset);
-        UA_String_encodeBinary(&errorMsg,dst,&offset);
+        UA_UInt32_encodeBinary(&ci->errorCode, dst, &offset);
+        UA_String_encodeBinary(&errorMsg, dst, &offset);
     }
     respHeader.messageHeader.messageSize = (UA_UInt32)offset;
     ci->messageSizeSoFar += offset;
 
-    printf("send chunk of length %i\n", respHeader.messageHeader.messageSize);
-
+    /* Encode the header at the beginning of the buffer */
     UA_SymmetricAlgorithmSecurityHeader symSecHeader;
     symSecHeader.tokenId = channel->securityToken.tokenId;
-
     UA_SequenceHeader seqHeader;
     seqHeader.requestId = ci->requestId;
 #ifndef UA_ENABLE_MULTITHREADING
@@ -176,8 +175,6 @@ UA_SecureChannel_sendChunk(UA_ChunkInfo *ci, UA_ByteString *dst, size_t offset)
 #else
     seqHeader.sequenceNumber = uatomic_add_return(&channel->sendSequenceNumber, 1);
 #endif
-
-    /* Encode the header at the beginning of the buffer */
     size_t offset_header = 0;
     UA_SecureConversationMessageHeader_encodeBinary(&respHeader, dst, &offset_header);
     UA_SymmetricAlgorithmSecurityHeader_encodeBinary(&symSecHeader, dst, &offset_header);
@@ -188,7 +185,7 @@ UA_SecureChannel_sendChunk(UA_ChunkInfo *ci, UA_ByteString *dst, size_t offset)
     connection->send(channel->connection, dst);
 
     /* Replace with the buffer for the next chunk */
-    if(!ci->final && !ci->abort) {
+    if(!ci->final) {
         UA_StatusCode retval = connection->getSendBuffer(connection, connection->localConf.sendBufferSize, dst);
         if(retval != UA_STATUSCODE_GOOD)
             return retval;
@@ -196,9 +193,7 @@ UA_SecureChannel_sendChunk(UA_ChunkInfo *ci, UA_ByteString *dst, size_t offset)
         dst->data = &dst->data[UA_SECURE_MESSAGE_HEADER_LENGTH];
         dst->length = connection->localConf.sendBufferSize - UA_SECURE_MESSAGE_HEADER_LENGTH;
     }
-    if(ci->abort)
-        return UA_STATUSCODE_BADTCPMESSAGETOOLARGE;
-    return UA_STATUSCODE_GOOD;
+    return ci->errorCode;
 }
 
 UA_StatusCode
@@ -232,7 +227,7 @@ UA_SecureChannel_sendBinaryMessage(UA_SecureChannel *channel, UA_UInt32 requestI
     ci.messageSizeSoFar = 0;
     ci.final = false;
     ci.messageType = UA_MESSAGETYPE_MSG;
-    ci.abort = false;
+    ci.errorCode = UA_STATUSCODE_GOOD;
     if(typeId.identifier.numeric == 446 || typeId.identifier.numeric == 449)
         ci.messageType = UA_MESSAGETYPE_OPN;
     else if(typeId.identifier.numeric == 452 || typeId.identifier.numeric == 455)
@@ -240,15 +235,16 @@ UA_SecureChannel_sendBinaryMessage(UA_SecureChannel *channel, UA_UInt32 requestI
     retval = UA_encodeBinary(content, contentType, (UA_exchangeEncodeBuffer)UA_SecureChannel_sendChunk,
                              &ci, &message, &messagePos);
 
-    /* Abort message was sent, the buffer is already freed */
-    if(ci.abort)
-        return retval;
-
     /* Encoding failed, release the message */
     if(retval != UA_STATUSCODE_GOOD) {
-        /* Unhide the beginning of the buffer (header) */
-        message.data = &message.data[-UA_SECURE_MESSAGE_HEADER_LENGTH];
-        connection->releaseSendBuffer(connection, &message);
+        if(!ci.final) {
+            ci.errorCode = retval;
+            UA_SecureChannel_sendChunk(&ci, &message, messagePos);
+        } else {
+            /* Unhide the beginning of the buffer (header) */
+            message.data = &message.data[-UA_SECURE_MESSAGE_HEADER_LENGTH];
+            connection->releaseSendBuffer(connection, &message);
+        }
         return retval;
     }
 

src/ua_securechannel.h

@@ -29,7 +29,7 @@ typedef struct {
     UA_UInt16 chunksSoFar;
     size_t messageSizeSoFar;
     UA_Boolean final;
-    UA_Boolean abort;
+    UA_StatusCode errorCode;
 } UA_ChunkInfo;
 
 struct UA_SecureChannel {

src/ua_types_encoding_binary.c

@@ -12,21 +12,38 @@ static const UA_decodeBinarySignature decodeBinaryJumpTable[UA_BUILTIN_TYPES_COU
 typedef size_t (*UA_calcSizeBinarySignature)(const void *UA_RESTRICT p, const UA_DataType *contenttype);
 static const UA_calcSizeBinarySignature calcSizeBinaryJumpTable[UA_BUILTIN_TYPES_COUNT + 1];
 
-/* Thread-local buffers used for exchanging the buffer for chunking */
-UA_THREAD_LOCAL UA_ByteString *encodeBuf; /* the original buffer */
-UA_THREAD_LOCAL UA_exchangeEncodeBuffer exchangeBufferCallback;
-UA_THREAD_LOCAL void *exchangeBufferCallbackHandle;
-
 /* We give pointers to the current position and the last position in the buffer
    instead of a string with an offset. */
 UA_THREAD_LOCAL UA_Byte * pos;
 UA_THREAD_LOCAL UA_Byte * end;
 
+/* The code UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED is returned only when the
+ * end of the buffer is reached. This error is caught. We then try to send the
+ * current chunk and continue with the next. */
+
+/* Thread-local buffers used for exchanging the buffer for chunking */
+UA_THREAD_LOCAL UA_ByteString *encodeBuf; /* the original buffer */
+UA_THREAD_LOCAL UA_exchangeEncodeBuffer exchangeBufferCallback;
+UA_THREAD_LOCAL void *exchangeBufferCallbackHandle;
+
+/* Send the current chunk and replace the buffer */
 static UA_StatusCode exchangeBuffer(void) {
     if(!exchangeBufferCallback)
         return UA_STATUSCODE_BADENCODINGERROR;
+
+    /* store context variables since chunk-sending might call UA_encode itself */
+    UA_ByteString *store_encodeBuf = encodeBuf;
+    UA_exchangeEncodeBuffer store_exchangeBufferCallback = exchangeBufferCallback;
+    void *store_exchangeBufferCallbackHandle = exchangeBufferCallbackHandle;
+
     size_t offset = ((uintptr_t)pos - (uintptr_t)encodeBuf->data) / sizeof(UA_Byte);
     UA_StatusCode retval = exchangeBufferCallback(exchangeBufferCallbackHandle, encodeBuf, offset);
+
+    /* restore context variables */
+    encodeBuf = store_encodeBuf;
+    exchangeBufferCallback = store_exchangeBufferCallback;
+    exchangeBufferCallbackHandle = store_exchangeBufferCallbackHandle;
+
     /* set pos and end in order to continue encoding */
     pos = encodeBuf->data;
     end = &encodeBuf->data[encodeBuf->length];
@@ -70,7 +87,7 @@ static void UA_decode64(const UA_Byte buf[8], UA_UInt64 *v) {
 static UA_StatusCode
 Boolean_encodeBinary(const UA_Boolean *src, const UA_DataType *_) {
     if(pos + sizeof(UA_Boolean) > end)
-        return UA_STATUSCODE_BADENCODINGERROR;
+        return UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
     *pos = *(const UA_Byte*)src;
     pos++;
     return UA_STATUSCODE_GOOD;
@@ -89,7 +106,7 @@ Boolean_decodeBinary(UA_Boolean *dst, const UA_DataType *_) {
 static UA_StatusCode
 Byte_encodeBinary(const UA_Byte *src, const UA_DataType *_) {
     if(pos + sizeof(UA_Byte) > end)
-        return UA_STATUSCODE_BADENCODINGERROR;
+        return UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
     *pos = *(const UA_Byte*)src;
     pos++;
     return UA_STATUSCODE_GOOD;
@@ -108,7 +125,7 @@ Byte_decodeBinary(UA_Byte *dst, const UA_DataType *_) {
 static UA_StatusCode
 UInt16_encodeBinary(UA_UInt16 const *src, const UA_DataType *_) {
     if(pos + sizeof(UA_UInt16) > end)
-        return UA_STATUSCODE_BADENCODINGERROR;
+        return UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
 #if UA_BINARY_OVERLAYABLE_INTEGER
     memcpy(pos, src, sizeof(UA_UInt16));
 #else
@@ -137,15 +154,13 @@ UInt16_decodeBinary(UA_UInt16 *dst, const UA_DataType *_) {
 }
 
 static UA_INLINE UA_StatusCode
-Int16_decodeBinary(UA_Int16 *dst) {
-    return UInt16_decodeBinary((UA_UInt16*)dst, NULL);
-}
+Int16_decodeBinary(UA_Int16 *dst) { return UInt16_decodeBinary((UA_UInt16*)dst, NULL); }
 
 /* UInt32 */
 static UA_StatusCode
 UInt32_encodeBinary(UA_UInt32 const *src, const UA_DataType *_) {
     if(pos + sizeof(UA_UInt32) > end)
-        return UA_STATUSCODE_BADENCODINGERROR;
+        return UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
 #if UA_BINARY_OVERLAYABLE_INTEGER
     memcpy(pos, src, sizeof(UA_UInt32));
 #else
@@ -184,7 +199,7 @@ StatusCode_decodeBinary(UA_StatusCode *dst) { return UInt32_decodeBinary((UA_UIn
 static UA_StatusCode
 UInt64_encodeBinary(UA_UInt64 const *src, const UA_DataType *_) {
     if(pos + sizeof(UA_UInt64) > end)
-        return UA_STATUSCODE_BADENCODINGERROR;
+        return UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
 #if UA_BINARY_OVERLAYABLE_INTEGER
     memcpy(pos, src, sizeof(UA_UInt64));
 #else
@@ -377,7 +392,7 @@ Array_encodeBinary(const void *src, size_t length, const UA_DataType *type) {
         UA_Byte *oldpos = pos;
         retval = encodeBinaryJumpTable[encode_index]((const void*)ptr, type);
         ptr += type->memSize;
-        if(retval == UA_STATUSCODE_BADENCODINGERROR) {
+        if(retval == UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED) {
             /* exchange the buffer and try to encode the same element once more */
             pos = oldpos;
             retval = exchangeBuffer();
@@ -443,34 +458,18 @@ String_encodeBinary(UA_String const *src, const UA_DataType *_) {
     return Array_encodeBinary(src->data, src->length, &UA_TYPES[UA_TYPES_BYTE]);
 }
 
-static UA_INLINE UA_StatusCode
-ByteString_encodeBinary(UA_ByteString const *src) { return String_encodeBinary((const UA_String*)src, NULL); }
-
 static UA_StatusCode
 String_decodeBinary(UA_String *dst, const UA_DataType *_) {
     UA_Int32 signed_length;
     UA_StatusCode retval = Int32_decodeBinary(&signed_length);
     if(retval != UA_STATUSCODE_GOOD)
-        return UA_STATUSCODE_BADINTERNALERROR;
-    if(signed_length <= 0) {
-        if(signed_length == 0)
-            dst->data = UA_EMPTY_ARRAY_SENTINEL;
-        else
-            dst->data = NULL;
-        return UA_STATUSCODE_GOOD;
-    }
-    size_t length = (size_t)signed_length;
-    if(pos + length > end)
-        return UA_STATUSCODE_BADDECODINGERROR;
-    dst->data = UA_malloc(length);
-    if(!dst->data)
-        return UA_STATUSCODE_BADOUTOFMEMORY;
-    memcpy(dst->data, pos, length);
-    dst->length = length;
-    pos += length;
-    return UA_STATUSCODE_GOOD;
+        return retval;
+    return Array_decodeBinary(signed_length, (void**)&dst->data, &dst->length, &UA_TYPES[UA_TYPES_BYTE]);
 }
 
+static UA_INLINE UA_StatusCode
+ByteString_encodeBinary(UA_ByteString const *src) { return String_encodeBinary((const UA_String*)src, NULL); }
+
 static UA_INLINE UA_StatusCode
 ByteString_decodeBinary(UA_ByteString *dst) { return String_decodeBinary((UA_ByteString*)dst, NULL); }
 
@@ -480,8 +479,10 @@ Guid_encodeBinary(UA_Guid const *src, const UA_DataType *_) {
     UA_StatusCode retval = UInt32_encodeBinary(&src->data1, NULL);
     retval |= UInt16_encodeBinary(&src->data2, NULL);
     retval |= UInt16_encodeBinary(&src->data3, NULL);
-    for(UA_Int32 i = 0; i < 8; i++)
-        retval |= Byte_encodeBinary(&src->data4[i], NULL);
+    if(pos + (8*sizeof(UA_Byte)) > end)
+        return UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
+    memcpy(pos, src->data4, 8*sizeof(UA_Byte));
+    pos += 8;
     return retval;
 }
 
@@ -490,10 +491,10 @@ Guid_decodeBinary(UA_Guid *dst, const UA_DataType *_) {
     UA_StatusCode retval = UInt32_decodeBinary(&dst->data1, NULL);
     retval |= UInt16_decodeBinary(&dst->data2, NULL);
     retval |= UInt16_decodeBinary(&dst->data3, NULL);
-    for(size_t i = 0; i < 8; i++)
-        retval |= Byte_decodeBinary(&dst->data4[i], NULL);
-    if(retval != UA_STATUSCODE_GOOD)
-        UA_Guid_deleteMembers(dst);
+    if(pos + (8*sizeof(UA_Byte)) > end)
+        return UA_STATUSCODE_BADDECODINGERROR;
+    memcpy(dst->data4, pos, 8*sizeof(UA_Byte));
+    pos += 8;
     return retval;
 }
 
@@ -612,7 +613,7 @@ NodeId_decodeBinary(UA_NodeId *dst, const UA_DataType *_) {
 static UA_StatusCode
 ExpandedNodeId_encodeBinary(UA_ExpandedNodeId const *src, const UA_DataType *_) {
     if(pos >= end)
-        return UA_STATUSCODE_BADENCODINGERROR;
+        return UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
     UA_Byte *start = pos;
     UA_StatusCode retval = NodeId_encodeBinary(&src->nodeId, NULL);
     if(src->namespaceUri.length > 0) {
@@ -838,7 +839,7 @@ Variant_encodeBinary(UA_Variant const *src, const UA_DataType *_) {
             eo.content.decoded.data = (void*)ptr;
             retval |= ExtensionObject_encodeBinary(&eo, NULL);
             ptr += memSize;
-            if(retval == UA_STATUSCODE_BADENCODINGERROR) {
+            if(retval == UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED) {
                 /* exchange/send with the current buffer with chunking */
                 pos = oldpos;
                 retval = exchangeBuffer();
@@ -851,8 +852,8 @@ Variant_encodeBinary(UA_Variant const *src, const UA_DataType *_) {
 
     /* Encode the dimensions */
     if(hasDimensions)
-        retval |= Array_encodeBinary(src->arrayDimensions, src->arrayDimensionsSize,
-                                     &UA_TYPES[UA_TYPES_INT32]);
+        retval |= Array_encodeBinary(src->arrayDimensions, src->arrayDimensionsSize, &UA_TYPES[UA_TYPES_INT32]);
+
     return retval;
 }
 
@@ -936,6 +937,7 @@ Variant_decodeBinary(UA_Variant *dst, const UA_DataType *_) {
             retval = Array_decodeBinary(signed_length, (void**)&dst->arrayDimensions,
                                         &dst->arrayDimensionsSize, &UA_TYPES[UA_TYPES_INT32]);
     }
+
     if(retval != UA_STATUSCODE_GOOD)
         UA_Variant_deleteMembers(dst);
     return retval;
@@ -1094,7 +1096,7 @@ UA_encodeBinaryInternal(const void *src, const UA_DataType *type) {
             UA_Byte *oldpos = pos;
             retval |= encodeBinaryJumpTable[encode_index]((const void*)ptr, membertype);
             ptr += memSize;
-            if(retval == UA_STATUSCODE_BADENCODINGERROR) {
+            if(retval == UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED) {
                 /* exchange/send the buffer and try to encode the same type once more */
                 pos = oldpos;
                 retval = exchangeBuffer();