fix(core): Fix UA_String_format when the string needs to be allocated internally

deps/mp_printf.c

@@ -591,17 +591,15 @@ format_string_loop(output_t *output, const char *format, va_list args) {
 
 int
 mp_vsnprintf(char *s, size_t n, const char *format, va_list arg) {
-    // Check that the inputs are sane
-    if(!s || n < 1)
-        return -1;
-
     // Format the string
     output_t out = {s, 0, n};
     format_string_loop(&out, format, arg);
 
     // Write the string-terminating '\0' character
-    size_t null_char_pos = out.pos < out.max_chars ? out.pos : out.max_chars - 1;
+    if(n > 1) {
-    out.buffer[null_char_pos] = '\0';
+        size_t null_char_pos = out.pos < n ? out.pos : n - 1;
+        out.buffer[null_char_pos] = '\0';
+    }
 
     // Return written chars without terminating \0
     return (int)out.pos;

src/ua_types.c

@@ -207,35 +207,48 @@ UA_String_format(UA_String *str, const char *format, ...) {
 
 UA_StatusCode
 UA_String_vformat(UA_String *str, const char *format, va_list args) {
+    /* Store a copy of the arguments for the second pass. va_list cannot be
+     * iterated twice. */
+    va_list args2;
+    va_copy(args2, args);
+
     /* Encode initially */
+    UA_StatusCode res = UA_STATUSCODE_GOOD;
     int out = mp_vsnprintf((char*)str->data, str->length, format, args);
-    if(out < 0)
+    if(out < 0) {
-        return UA_STATUSCODE_BADENCODINGERROR;
+        res = UA_STATUSCODE_BADENCODINGERROR;
+        goto errout;
+    }
 
     /* Output length zero */
     if(out == 0) {
         str->length = 0;
         if(str->data == NULL)
             str->data = (UA_Byte*)UA_EMPTY_ARRAY_SENTINEL;
-        return UA_STATUSCODE_GOOD;
+        goto errout;
     }
 
     /* Encode into existing buffer. mp_snprintf adds a trailing \0. So out must
      * be truly smaller than str->length for success. */
     if(str->length > 0) {
-        if((size_t)out >= str->length)
+        if((size_t)out < str->length) {
-            return UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
+            str->length = (size_t)out;
-        str->length = (size_t)out;
+        } else {
-        return UA_STATUSCODE_GOOD;
+            res = UA_STATUSCODE_BADENCODINGLIMITSEXCEEDED;
+        }
+        goto errout;
     }
 
     /* Allocate and encode again (+1 length for the trailing \0) */
-    UA_StatusCode res = UA_ByteString_allocBuffer(str, (size_t)out + 1);
+    res = UA_ByteString_allocBuffer(str, (size_t)out + 1);
     if(res != UA_STATUSCODE_GOOD)
-        return res;
+        goto errout;
-    mp_vsnprintf((char*)str->data, str->length, format, args);
+    mp_vsnprintf((char*)str->data, str->length, format, args2);
     str->length--;
-    return UA_STATUSCODE_GOOD;
+
+ errout:
+    va_end(args2);
+    return res;
 }
 
 /* QualifiedName */

tests/check_utils.c

@@ -761,6 +761,25 @@ START_TEST(qualifiedNameNsIndex) {
     UA_String_clear(&str);
 } END_TEST
 
+START_TEST(format_string) {
+    UA_NodeId test = UA_NODEID_NUMERIC(1,1);
+    UA_String testStr = UA_STRING("banana");
+    UA_String out = UA_STRING_NULL;
+
+    UA_StatusCode res = UA_String_format(&out, "test %N %S", test, testStr);
+    ck_assert_uint_eq(res, UA_STATUSCODE_GOOD);
+
+    UA_String expected = UA_STRING("test ns=1;i=1 banana");
+    ck_assert(UA_String_equal(&out, &expected));
+
+    UA_String_clear(&out);
+
+    UA_Byte buf[4];
+    UA_String shortOut = {4, buf};
+    res = UA_String_format(&shortOut, "test %N %S", test, testStr);
+    ck_assert_uint_ne(res, UA_STATUSCODE_GOOD);
+} END_TEST
+
 static Suite* testSuite_Utils(void) {
     Suite *s = suite_create("Utils");
     TCase *tc_endpointUrl_split = tcase_create("EndpointUrl_split");
@@ -808,6 +827,10 @@ static Suite* testSuite_Utils(void) {
     tcase_add_test(tc5, qualifiedNameNsIndex);
     suite_add_tcase(s, tc5);
 
+    TCase *tc6 = tcase_create("test string format");
+    tcase_add_test(tc6, format_string);
+    suite_add_tcase(s, tc6);
+
     return s;
 }