check the result of MonitoredItem_registerSampleCallback and setSubscriptionSettings

include/ua_server_config.h

@@ -146,7 +146,7 @@ struct UA_ServerConfig {
 
     /* Limits for Subscriptions */
     UA_UInt32 maxSubscriptionsPerSession;
-    UA_DurationRange publishingIntervalLimits;
+    UA_DurationRange publishingIntervalLimits; /* in ms (must not be less than 5) */
     UA_UInt32Range lifeTimeCountLimits;
     UA_UInt32Range keepAliveCountLimits;
     UA_UInt32 maxNotificationsPerPublish;
@@ -157,7 +157,7 @@ struct UA_ServerConfig {
 
     /* Limits for MonitoredItems */
     UA_UInt32 maxMonitoredItemsPerSubscription;
-    UA_DurationRange samplingIntervalLimits;
+    UA_DurationRange samplingIntervalLimits; /* in ms (must not be less than 5) */
     UA_UInt32Range queueSizeLimits; /* Negotiated with the client */
 
     /* Limits for PublishRequests */

src/server/ua_services_subscription.c

@@ -27,7 +27,7 @@
         else DST = SRC;                                \
     }
 
-static void
+static UA_StatusCode
 setSubscriptionSettings(UA_Server *server, UA_Subscription *subscription,
                         UA_Double requestedPublishingInterval,
                         UA_UInt32 requestedLifetimeCount,
@@ -35,10 +35,12 @@ setSubscriptionSettings(UA_Server *server, UA_Subscription *subscription,
                         UA_UInt32 maxNotificationsPerPublish, UA_Byte priority) {
     /* deregister the callback if required */
     UA_StatusCode retval = Subscription_unregisterPublishCallback(server, subscription);
-    if(retval != UA_STATUSCODE_GOOD)
+    if(retval != UA_STATUSCODE_GOOD) {
         UA_LOG_DEBUG_SESSION(server->config.logger, subscription->session,
                              "Subscription %u | Could not unregister publish callback with error code %s",
                              subscription->subscriptionId, UA_StatusCode_name(retval));
+        return retval;
+    }
 
     /* re-parameterize the subscription */
     subscription->publishingInterval = requestedPublishingInterval;
@@ -60,10 +62,13 @@ setSubscriptionSettings(UA_Server *server, UA_Subscription *subscription,
     subscription->priority = priority;
 
     retval = Subscription_registerPublishCallback(server, subscription);
-    if(retval != UA_STATUSCODE_GOOD)
+    if(retval != UA_STATUSCODE_GOOD) {
         UA_LOG_DEBUG_SESSION(server->config.logger, subscription->session,
                              "Subscription %u | Could not register publish callback with error code %s",
                              subscription->subscriptionId, UA_StatusCode_name(retval));
+        return retval;
+    }
+    return UA_STATUSCODE_GOOD;
 }
 
 void
@@ -90,9 +95,15 @@ Service_CreateSubscription(UA_Server *server, UA_Session *session,
 
     /* Set the subscription parameters */
     newSubscription->publishingEnabled = request->publishingEnabled;
-    setSubscriptionSettings(server, newSubscription, request->requestedPublishingInterval,
-                            request->requestedLifetimeCount, request->requestedMaxKeepAliveCount,
-                            request->maxNotificationsPerPublish, request->priority);
+    UA_StatusCode retval = setSubscriptionSettings(server, newSubscription, request->requestedPublishingInterval,
+                                                   request->requestedLifetimeCount, request->requestedMaxKeepAliveCount,
+                                                   request->maxNotificationsPerPublish, request->priority);
+
+    if(retval != UA_STATUSCODE_GOOD) {
+        response->responseHeader.serviceResult = retval;
+        return;
+    }
+
     newSubscription->currentKeepAliveCount = newSubscription->maxKeepAliveCount; /* set settings first */
 
     /* Prepare the response */
@@ -118,9 +129,15 @@ Service_ModifySubscription(UA_Server *server, UA_Session *session,
         return;
     }
 
-    setSubscriptionSettings(server, sub, request->requestedPublishingInterval,
-                            request->requestedLifetimeCount, request->requestedMaxKeepAliveCount,
-                            request->maxNotificationsPerPublish, request->priority);
+    UA_StatusCode retval = setSubscriptionSettings(server, sub, request->requestedPublishingInterval,
+                                                   request->requestedLifetimeCount, request->requestedMaxKeepAliveCount,
+                                                   request->maxNotificationsPerPublish, request->priority);
+
+    if(retval != UA_STATUSCODE_GOOD) {
+        response->responseHeader.serviceResult = retval;
+        return;
+    }
+
     sub->currentLifetimeCount = 0; /* Reset the subscription lifetime */
     response->revisedPublishingInterval = sub->publishingInterval;
     response->revisedLifetimeCount = sub->lifeTimeCount;
@@ -237,7 +254,8 @@ setMonitoredItemSettings(UA_Server *server, UA_MonitoredItem *mon,
     /* Register sample callback if reporting is enabled */
     mon->monitoringMode = monitoringMode;
     if(monitoringMode == UA_MONITORINGMODE_REPORTING)
-        UA_MonitoredItem_registerSampleCallback(server, mon);
+        return UA_MonitoredItem_registerSampleCallback(server, mon);
+
     return UA_STATUSCODE_GOOD;
 }
 
@@ -542,7 +560,7 @@ Operation_SetMonitoringMode(UA_Server *server, UA_Session *session,
 
     mon->monitoringMode = smc->monitoringMode;
     if(mon->monitoringMode == UA_MONITORINGMODE_REPORTING) {
-        UA_MonitoredItem_registerSampleCallback(server, mon);
+        *result = UA_MonitoredItem_registerSampleCallback(server, mon);
     } else {
         UA_MonitoredItem_unregisterSampleCallback(server, mon);
 

tests/server/check_services_subscriptions.c

@@ -730,6 +730,66 @@ START_TEST(Server_lifeTimeCount) {
 }
 END_TEST
 
+START_TEST(Server_invalidPublishingInterval) {
+    UA_Double savedPublishingIntervalLimitsMin = server->config.publishingIntervalLimits.min;
+    server->config.publishingIntervalLimits.min = 1;
+    /* Create a subscription */
+    UA_CreateSubscriptionRequest request;
+    UA_CreateSubscriptionResponse response;
+
+    UA_CreateSubscriptionRequest_init(&request);
+    request.publishingEnabled = true;
+    request.requestedPublishingInterval = 1; // Must be < 5
+printf("BOFFF1 %f\n", server->config.publishingIntervalLimits.min);
+    UA_CreateSubscriptionResponse_init(&response);
+    Service_CreateSubscription(server, session, &request, &response);
+    ck_assert_uint_eq(response.responseHeader.serviceResult, UA_STATUSCODE_BADINTERNALERROR);
+    UA_CreateSubscriptionResponse_deleteMembers(&response);
+
+    server->config.publishingIntervalLimits.min = savedPublishingIntervalLimitsMin;
+}
+END_TEST
+
+START_TEST(Server_invalidSamplingInterval) {
+    createSubscription();
+
+    UA_Double savedSamplingIntervalLimitsMin = server->config.samplingIntervalLimits.min;
+    server->config.samplingIntervalLimits.min = 1;
+
+    UA_CreateMonitoredItemsRequest request;
+    UA_CreateMonitoredItemsRequest_init(&request);
+    request.subscriptionId = subscriptionId;
+    request.timestampsToReturn = UA_TIMESTAMPSTORETURN_SERVER;
+    UA_MonitoredItemCreateRequest item;
+    UA_MonitoredItemCreateRequest_init(&item);
+    UA_ReadValueId rvi;
+    UA_ReadValueId_init(&rvi);
+    rvi.nodeId = UA_NODEID_NUMERIC(0, UA_NS0ID_SERVER);
+    rvi.attributeId = UA_ATTRIBUTEID_BROWSENAME;
+    rvi.indexRange = UA_STRING_NULL;
+    item.itemToMonitor = rvi;
+    item.monitoringMode = UA_MONITORINGMODE_REPORTING;
+    UA_MonitoringParameters params;
+    UA_MonitoringParameters_init(&params);
+    params.samplingInterval = 1; // Must be < 5
+    item.requestedParameters = params;
+    request.itemsToCreateSize = 1;
+    request.itemsToCreate = &item;
+
+    UA_CreateMonitoredItemsResponse response;
+    UA_CreateMonitoredItemsResponse_init(&response);
+    Service_CreateMonitoredItems(server, session, &request, &response);
+    ck_assert_uint_eq(response.responseHeader.serviceResult, UA_STATUSCODE_GOOD);
+    ck_assert_uint_eq(response.resultsSize, 1);
+    ck_assert_uint_eq(response.results[0].statusCode, UA_STATUSCODE_BADINTERNALERROR);
+
+    UA_MonitoredItemCreateRequest_deleteMembers(&item);
+    UA_CreateMonitoredItemsResponse_deleteMembers(&response);
+
+    server->config.samplingIntervalLimits.min = savedSamplingIntervalLimitsMin;
+}
+END_TEST
+
 #endif /* UA_ENABLE_SUBSCRIPTIONS */
 
 static Suite* testSuite_Client(void) {
@@ -740,16 +800,18 @@ static Suite* testSuite_Client(void) {
     tcase_add_test(tc_server, Server_createSubscription);
     tcase_add_test(tc_server, Server_modifySubscription);
     tcase_add_test(tc_server, Server_setPublishingMode);
+    tcase_add_test(tc_server, Server_invalidSamplingInterval);
     tcase_add_test(tc_server, Server_createMonitoredItems);
     tcase_add_test(tc_server, Server_modifyMonitoredItems);
     tcase_add_test(tc_server, Server_overflow);
     tcase_add_test(tc_server, Server_setMonitoringMode);
     tcase_add_test(tc_server, Server_deleteMonitoredItems);
     tcase_add_test(tc_server, Server_republish);
-    tcase_add_test(tc_server, Server_deleteSubscription);
     tcase_add_test(tc_server, Server_republish_invalid);
+    tcase_add_test(tc_server, Server_deleteSubscription);
     tcase_add_test(tc_server, Server_publishCallback);
     tcase_add_test(tc_server, Server_lifeTimeCount);
+    tcase_add_test(tc_server, Server_invalidPublishingInterval);
 #endif /* UA_ENABLE_SUBSCRIPTIONS */
     suite_add_tcase(s, tc_server);