Browse Source

fix possible infinity loops and add a unit test unit

StalderT 7 years ago
parent
commit
3948143c50

+ 8 - 8
plugins/ua_network_tcp.c

@@ -169,13 +169,6 @@ connection_write(UA_Connection *connection, UA_ByteString *buf) {
 static UA_StatusCode
 connection_recv(UA_Connection *connection, UA_ByteString *response,
                 UA_UInt32 timeout) {
-    response->data = (UA_Byte*)
-        UA_malloc(connection->localConf.recvBufferSize);
-    if(!response->data) {
-        response->length = 0;
-        return UA_STATUSCODE_BADOUTOFMEMORY; /* not enough memory retry */
-    }
-
     /* Listen on the socket for the given timeout until a message arrives */
     if(timeout > 0) {
         fd_set fdset;
@@ -189,7 +182,14 @@ connection_recv(UA_Connection *connection, UA_ByteString *response,
 
         /* No result */
         if(resultsize == 0)
-            return UA_STATUSCODE_GOOD;
+            return UA_STATUSCODE_GOODNONCRITICALTIMEOUT;
+    }
+
+    response->data = (UA_Byte*)
+        UA_malloc(connection->localConf.recvBufferSize);
+    if(!response->data) {
+        response->length = 0;
+        return UA_STATUSCODE_BADOUTOFMEMORY; /* not enough memory retry */
     }
 
     /* Get the received packet(s) */

+ 15 - 4
src/client/ua_client.c

@@ -269,13 +269,22 @@ receiveServiceResponse(UA_Client *client, void *response, const UA_DataType *res
     UA_StatusCode retval;
     do {
         UA_DateTime now = UA_DateTime_nowMonotonic();
-        if(now > maxDate)
+
+        /* >= avoid timeout to be set to 0 */
+        if(now >= maxDate)
             return UA_STATUSCODE_GOODNONCRITICALTIMEOUT;
-        UA_UInt32 timeout = (UA_UInt32)((maxDate - now) / UA_MSEC_TO_DATETIME);
+
+        /* round always to upper value to avoid timeout to be set to 0
+         * if (maxDate - now) < (UA_MSEC_TO_DATETIME/2) */
+        UA_UInt32 timeout = (UA_UInt32)(((maxDate - now) + (UA_MSEC_TO_DATETIME - 1)) / UA_MSEC_TO_DATETIME);
+
         retval = UA_Connection_receiveChunksBlocking(&client->connection, &rd,
                                                      client_processChunk, timeout);
 
-        if(retval != UA_STATUSCODE_GOOD && retval != UA_STATUSCODE_GOODNONCRITICALTIMEOUT) {
+        if (retval == UA_STATUSCODE_GOODNONCRITICALTIMEOUT)
+            break;
+
+        if(retval != UA_STATUSCODE_GOOD) {
             if(retval == UA_STATUSCODE_BADCONNECTIONCLOSED)
                 client->state = UA_CLIENTSTATE_DISCONNECTED;
             else
@@ -309,8 +318,10 @@ __UA_Client_Service(UA_Client *client, const void *request,
     UA_DateTime maxDate = UA_DateTime_nowMonotonic() +
         (client->config.timeout * UA_MSEC_TO_DATETIME);
     retval = receiveServiceResponse(client, response, responseType, maxDate, &requestId);
-    if(retval != UA_STATUSCODE_GOOD)
+    if(retval != UA_STATUSCODE_GOOD){
         respHeader->serviceResult = retval;
+        UA_Client_disconnect(client);
+    }
 }
 
 UA_StatusCode

+ 4 - 1
src/server/ua_server_worker.c

@@ -326,8 +326,11 @@ UA_Server_run_iterate(UA_Server *server, UA_Boolean waitInternal) {
         nextRepeated = latest;
 
     UA_UInt16 timeout = 0;
+
+    /* round always to upper value to avoid timeout to be set to 0
+    * if (nextRepeated - now) < (UA_MSEC_TO_DATETIME/2) */
     if(waitInternal)
-        timeout = (UA_UInt16)((nextRepeated - now) / UA_MSEC_TO_DATETIME);
+        timeout = (UA_UInt16)(((nextRepeated - now) + (UA_MSEC_TO_DATETIME - 1)) / UA_MSEC_TO_DATETIME);
 
     /* Listen on the networklayer */
     for(size_t i = 0; i < server->config.networkLayersSize; ++i) {

+ 7 - 2
src/ua_connection.c

@@ -217,9 +217,14 @@ UA_Connection_receiveChunksBlocking(UA_Connection *connection, void *application
         /* We received a message. But the chunk is incomplete. Compute the
          * remaining timeout. */
         now = UA_DateTime_nowMonotonic();
-        if(now > maxDate)
+
+        /* >= avoid timeout to be set to 0 */
+        if(now >= maxDate)
             return UA_STATUSCODE_GOODNONCRITICALTIMEOUT;
-        timeout = (UA_UInt32)((maxDate - now) / UA_MSEC_TO_DATETIME);
+
+        /* round always to upper value to avoid timeout to be set to 0
+         * if (maxDate - now) < (UA_MSEC_TO_DATETIME/2) */
+        timeout = (UA_UInt32)(((maxDate - now) + (UA_MSEC_TO_DATETIME - 1)) / UA_MSEC_TO_DATETIME);
     }
     return retval;
 }

+ 34 - 2
tests/client/check_client_securechannel.c

@@ -16,17 +16,24 @@
 UA_Server *server;
 UA_ServerConfig *config;
 UA_Boolean *running;
+UA_Boolean *blockServer;
 THREAD_HANDLE server_thread;
 
 THREAD_CALLBACK(serverloop) {
-    while(*running)
-        UA_Server_run_iterate(server, true);
+    while(*running){
+        if (*blockServer)
+            UA_realSleep(100);
+        else
+            UA_Server_run_iterate(server, true);
+    }
     return 0;
 }
 
 static void setup(void) {
     running = UA_Boolean_new();
     *running = true;
+    blockServer = UA_Boolean_new();
+    *blockServer = false;
     config = UA_ServerConfig_new_default();
     server = UA_Server_new(config);
     UA_Server_run_startup(server);
@@ -39,6 +46,7 @@ static void teardown(void) {
     THREAD_JOIN(server_thread);
     UA_Server_run_shutdown(server);
     UA_Boolean_delete(running);
+    UA_Boolean_delete(blockServer);
     UA_Server_delete(server);
     UA_ServerConfig_delete(config);
 }
@@ -82,12 +90,36 @@ START_TEST(SecureChannel_timeout_fail) {
 }
 END_TEST*/
 
+START_TEST(SecureChannel_networkfail) {
+    UA_Client *client = UA_Client_new(UA_ClientConfig_default);
+    UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+
+    *blockServer = true;
+    UA_realSleep(100);
+
+    UA_Variant val;
+    UA_Variant_init(&val);
+    UA_NodeId nodeId = UA_NODEID_NUMERIC(0, UA_NS0ID_SERVER_SERVERSTATUS_STATE);
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert(retval != UA_STATUSCODE_GOOD);
+
+    UA_Variant_deleteMembers(&val);
+
+    *blockServer = false;
+
+    UA_Client_disconnect(client);
+    UA_Client_delete(client);
+}
+END_TEST
+
 int main(void) {
     TCase *tc_sc = tcase_create("Client SecureChannel");
     tcase_add_checked_fixture(tc_sc, setup, teardown);
     tcase_add_test(tc_sc, SecureChannel_timeout_max);
     // Temporarily disable test since it is failing. See #1388
     //tcase_add_test(tc_sc, SecureChannel_timeout_fail);
+    tcase_add_test(tc_sc, SecureChannel_networkfail);
 
     Suite *s = suite_create("Client");
     suite_add_tcase(s, tc_sc);