Connection: Simplify storage of incomplete chunks

Now the connection receive method preprends the last incomplete chunk
 in the received buffer. This was made necessary by the new chunk
buffering in the SecureChannel.

arch/ua_network_tcp.c

@@ -110,16 +110,18 @@ connection_recv(UA_Connection *connection, UA_ByteString *response,
         }
     }
 
-    response->data = (UA_Byte*)
-        UA_malloc(connection->config.recvBufferSize);
+    response->data = (UA_Byte*)UA_malloc(connection->config.recvBufferSize);
     if(!response->data) {
         response->length = 0;
         return UA_STATUSCODE_BADOUTOFMEMORY; /* not enough memory retry */
     }
 
+    size_t offset = connection->incompleteChunk.length;
+    size_t remaining = connection->config.recvBufferSize - offset;
+
     /* Get the received packet(s) */
-    ssize_t ret = UA_recv(connection->sockfd, (char*)response->data,
-                          connection->config.recvBufferSize, 0);
+    ssize_t ret = UA_recv(connection->sockfd, (char*)&response->data[offset],
+                          remaining, 0);
 
     /* The remote side closed the connection */
     if(ret == 0) {
@@ -138,8 +140,13 @@ connection_recv(UA_Connection *connection, UA_ByteString *response,
         return UA_STATUSCODE_BADCONNECTIONCLOSED;
     }
 
+    /* Preprend the last incompleteChunk into the buffer */
+    memcpy(response->data, connection->incompleteChunk.data,
+           connection->incompleteChunk.length);
+    UA_ByteString_deleteMembers(&connection->incompleteChunk);
+
     /* Set the length of the received buffer */
-    response->length = (size_t)ret;
+    response->length = offset + (size_t)ret;
     return UA_STATUSCODE_GOOD;
 }
 

include/ua_plugin_network.h

@@ -68,7 +68,7 @@ struct UA_Connection {
                                       * simplifies the design. */
     UA_DateTime openingDate;         /* The date the connection was created */
     void *handle;                    /* A pointer to internal data */
-    UA_ByteString incompleteMessage; /* A half-received message (TCP is a
+    UA_ByteString incompleteChunk;   /* A half-received chunk (TCP is a
                                       * streaming protocol) is stored here */
     UA_UInt64 connectCallbackID;     /* Callback Id, for the connect-loop */
     /* Get a buffer for sending */

src/ua_connection.c

@@ -19,7 +19,7 @@
 #include "ua_securechannel.h"
 
 void UA_Connection_deleteMembers(UA_Connection *connection) {
-    UA_ByteString_deleteMembers(&connection->incompleteMessage);
+    UA_ByteString_deleteMembers(&connection->incompleteChunk);
 }
 
 UA_StatusCode
@@ -93,30 +93,14 @@ UA_Connection_sendError(UA_Connection *connection, UA_TcpErrorMessage *error) {
 }
 
 static UA_StatusCode
-prependIncompleteChunk(UA_Connection *connection, UA_ByteString *message) {
-    /* Allocate the new message buffer */
-    size_t length = connection->incompleteMessage.length + message->length;
-    UA_Byte *data = (UA_Byte*)UA_realloc(connection->incompleteMessage.data, length);
-    if(!data) {
-        UA_ByteString_deleteMembers(&connection->incompleteMessage);
-        return UA_STATUSCODE_BADOUTOFMEMORY;
-    }
-
-    /* Copy / release the current message buffer */
-    memcpy(&data[connection->incompleteMessage.length], message->data, message->length);
-    message->length = length;
-    message->data = data;
-    connection->incompleteMessage = UA_BYTESTRING_NULL;
-    return UA_STATUSCODE_GOOD;
-}
-
-static UA_StatusCode
-bufferIncompleteChunk(UA_Connection *connection, const UA_Byte *pos, size_t length) {
-    UA_assert(length > 0);
-    UA_StatusCode retval = UA_ByteString_allocBuffer(&connection->incompleteMessage, length);
+bufferIncompleteChunk(UA_Connection *connection, const UA_Byte *pos,
+                      const UA_Byte *end) {
+    UA_assert(pos < end);
+    size_t length = (uintptr_t)end - (uintptr_t)pos;
+    UA_StatusCode retval = UA_ByteString_allocBuffer(&connection->incompleteChunk, length);
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
-    memcpy(connection->incompleteMessage.data, pos, length);
+    memcpy(connection->incompleteChunk.data, pos, length);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -125,12 +109,10 @@ processChunk(UA_Connection *connection, void *application,
              UA_Connection_processChunk processCallback,
              const UA_Byte **posp, const UA_Byte *end, UA_Boolean *done) {
     const UA_Byte *pos = *posp;
-    const size_t length = (uintptr_t)end - (uintptr_t)pos;
+    const size_t remaining = (uintptr_t)end - (uintptr_t)pos;
 
     /* At least 8 byte needed for the header. Wait for the next chunk. */
-    if(length < 8) {
-        if(length > 0)
-            bufferIncompleteChunk(connection, pos, length);
+    if(remaining < 8) {
         *done = true;
         return UA_STATUSCODE_GOOD;
     }
@@ -161,9 +143,8 @@ processChunk(UA_Connection *connection, void *application,
     if(chunk_length < 16 || chunk_length > connection->config.recvBufferSize)
         return UA_STATUSCODE_BADTCPMESSAGETOOLARGE;
 
-    /* Wait for the next packet to process the complete chunk */
-    if(chunk_length > length) {
-        bufferIncompleteChunk(connection, pos, length);
+    /* Have an the complete chunk */
+    if(chunk_length > remaining) {
         *done = true;
         return UA_STATUSCODE_GOOD;
     }
@@ -179,30 +160,24 @@ UA_StatusCode
 UA_Connection_processChunks(UA_Connection *connection, void *application,
                             UA_Connection_processChunk processCallback,
                             const UA_ByteString *packet) {
-    /* If we have stored an incomplete chunk, prefix to the received message.
-     * After this block, connection->incompleteMessage is always empty. The
-     * message and the buffer is released if allocating the memory fails. */
-    UA_Boolean realloced = false;
-    UA_ByteString message = *packet;
-    UA_StatusCode retval;
-    if(connection->incompleteMessage.length > 0) {
-        retval = prependIncompleteChunk(connection, &message);
+    /* The connection has already prepended any incomplete chunk during recv */
+    UA_assert(connection->incompleteChunk.length == 0);
+
+    /* Loop over the received chunks. pos is increased with each chunk. */
+    const UA_Byte *pos = packet->data;
+    const UA_Byte *end = &packet->data[packet->length];
+    UA_Boolean done = false;
+    UA_StatusCode retval = UA_STATUSCODE_GOOD;
+    while(!done) {
+        retval = processChunk(connection, application, processCallback, &pos, end, &done);
+        /* If an irrecoverable error happens: do not buffer incomplete chunk */
         if(retval != UA_STATUSCODE_GOOD)
             return retval;
-        realloced = true;
     }
 
-    /* Loop over the received chunks. pos is increased with each chunk. */
-    const UA_Byte *pos = message.data;
-    const UA_Byte *end = &message.data[message.length];
-    UA_Boolean done = true;
-    do {
-        retval = processChunk(connection, application, processCallback,
-                              &pos, end, &done);
-    } while(!done && retval == UA_STATUSCODE_GOOD);
-
-    if(realloced)
-        UA_ByteString_deleteMembers(&message);
+    if(end > pos)
+        retval = bufferIncompleteChunk(connection, pos, end);
+
     return retval;
 }
 

tests/testing-plugins/testing_networklayers.c

@@ -60,7 +60,7 @@ UA_Connection createDummyConnection(size_t sendBufferSize,
     c.channel = NULL;
     c.sockfd = 0;
     c.handle = NULL;
-    c.incompleteMessage = UA_BYTESTRING_NULL;
+    c.incompleteChunk = UA_BYTESTRING_NULL;
     c.getSendBuffer = dummyGetSendBuffer;
     c.releaseSendBuffer = dummyReleaseSendBuffer;
     c.send = dummySend;