Added quoted argument support to CommandLineParseCommaSeparatedValuesEx

* Argument quoting support
* Empty list element detection
* Unit test for argument parser

winpr/libwinpr/utils/cmdline.c

@@ -483,24 +483,152 @@ const COMMAND_LINE_ARGUMENT_A* CommandLineFindNextArgumentA(const COMMAND_LINE_A
 	return nextArgument;
 }
 
+static size_t get_element_count(const char* list, BOOL* failed)
+{
+	size_t count = 0;
+	BOOL quoted = FALSE;
+	BOOL finished = FALSE;
+	BOOL first = TRUE;
+	const char* it = list;
+
+	if (!list)
+		return 0;
+	if (strlen(list) == 0)
+		return 0;
+
+	while (!finished)
+	{
+		BOOL nextFirst = FALSE;
+		switch (*it)
+		{
+			case '\0':
+				if (quoted)
+				{
+					WLog_ERR(TAG, "Invalid argument (missing closing quote) '%s'", list);
+					*failed = TRUE;
+					return 0;
+				}
+				finished = TRUE;
+				break;
+			case '"':
+				if (!quoted && !first)
+				{
+					WLog_ERR(TAG, "Invalid argument (misplaced quote) '%s'", list);
+					*failed = TRUE;
+					return 0;
+				}
+				quoted = !quoted;
+				break;
+			case ',':
+				if (first)
+				{
+					WLog_ERR(TAG, "Invalid argument (empty list elements) '%s'", list);
+					*failed = TRUE;
+					return 0;
+				}
+				if (!quoted)
+				{
+					nextFirst = TRUE;
+					count++;
+				}
+				break;
+			default:
+				break;
+		}
+
+		first = nextFirst;
+		it++;
+	}
+	return count + 1;
+}
+
+static char* get_next_comma(char* string)
+{
+	const char* log = string;
+	BOOL quoted = FALSE;
+	BOOL first = TRUE;
+
+	WINPR_ASSERT(string);
+
+	while (TRUE)
+	{
+		switch (*string)
+		{
+			case '\0':
+				if (quoted)
+					WLog_ERR(TAG, "Invalid quoted argument '%s'", log);
+				return NULL;
+
+			case '"':
+				if (!quoted && !first)
+				{
+					WLog_ERR(TAG, "Invalid quoted argument '%s'", log);
+					return NULL;
+				}
+				quoted = !quoted;
+				break;
+
+			case ',':
+				if (first)
+				{
+					WLog_ERR(TAG, "Invalid argument (empty list elements) '%s'", log);
+					return NULL;
+				}
+				if (!quoted)
+					return string;
+				break;
+
+			default:
+				break;
+		}
+		first = FALSE;
+		string++;
+	}
+
+	return NULL;
+}
+
 char** CommandLineParseCommaSeparatedValuesEx(const char* name, const char* list, size_t* count)
 {
-	char** p;
-	char* str;
-	size_t nArgs;
-	size_t index;
-	size_t nCommas;
-	size_t prefix, len, namelen = 0;
-	nCommas = 0;
+	char** p = NULL;
+	char* str = NULL;
+	size_t nArgs = 0;
+	size_t index = 0;
+	size_t prefix = 0;
+	size_t len = 0;
+	size_t namelen = 0;
+	BOOL failed = FALSE;
+	char* copy = NULL;
+	char* unquoted = NULL;
 
 	if (count == NULL)
-		return NULL;
+		goto fail;
 
 	*count = 0;
-
-	if (!list || strlen(list) == 0)
+	if (list)
 	{
-		if (name)
+		unquoted = copy = _strdup(list);
+		if (!copy)
+			goto fail;
+
+		len = strlen(unquoted);
+		if ((unquoted[0] == '"') && (unquoted[len - 1] == '"'))
+		{
+			unquoted[len - 1] = '\0';
+			unquoted++;
+			len -= 2;
+		}
+	}
+
+	*count = get_element_count(unquoted, &failed);
+	if (failed)
+		goto fail;
+
+	if (*count == 0)
+	{
+		if (!name)
+			goto fail;
+		else
 		{
 			size_t clen = strlen(name);
 			p = (char**)calloc(2UL + clen, sizeof(char*));
@@ -511,39 +639,26 @@ char** CommandLineParseCommaSeparatedValuesEx(const char* name, const char* list
 				p[0] = dst;
 				sprintf_s(dst, clen + 1, "%s", name);
 				*count = 1;
-				return p;
+				goto fail;
 			}
 		}
-
-		return NULL;
 	}
 
-	{
-		const char* it = list;
-
-		while ((it = strchr(it, ',')) != NULL)
-		{
-			it++;
-			nCommas++;
-		}
-	}
-
-	nArgs = nCommas + 1;
+	nArgs = *count;
 
 	if (name)
 		nArgs++;
 
 	prefix = (nArgs + 1UL) * sizeof(char*);
-	len = strlen(list);
 	if (name)
 		namelen = strlen(name);
 	p = (char**)calloc(len + prefix + 1 + namelen + 1, sizeof(char*));
 
 	if (!p)
-		return NULL;
+		goto fail;
 
 	str = &((char*)p)[prefix];
-	memcpy(str, list, len);
+	memcpy(str, unquoted, len);
 
 	if (name)
 	{
@@ -555,17 +670,28 @@ char** CommandLineParseCommaSeparatedValuesEx(const char* name, const char* list
 
 	for (index = name ? 1 : 0; index < nArgs; index++)
 	{
-		char* comma = strchr(str, ',');
-		p[index] = str;
+		char* ptr = str;
+		char* comma = get_next_comma(str);
+
+		if (*ptr == '"')
+			ptr++;
+
+		p[index] = ptr;
 
 		if (comma)
 		{
-			str = comma + 1;
+			char* last = comma - 1;
+			if (*last == '"')
+				*last = '\0';
 			*comma = '\0';
+
+			str = comma + 1;
 		}
 	}
 
 	*count = nArgs;
+fail:
+	free(copy);
 	return p;
 }
 

winpr/libwinpr/utils/test/TestCmdLine.c

@@ -1,5 +1,6 @@
 #include <errno.h>
 #include <winpr/crt.h>
+#include <winpr/assert.h>
 #include <winpr/tchar.h>
 #include <winpr/cmdline.h>
 #include <winpr/strlst.h>
@@ -18,6 +19,82 @@ static const char* testArgv[] = { "mstsc.exe",
 	                              "/valuelist-empty:",
 	                              0 };
 
+static const char testListAppName[] = "test app name";
+static const char* testListArgs[] = { "a,b,c,d", "a:,\"b:xxx, yyy\",c", "a:,,,b",
+	                                  "a:,\",b", "\"a,b,c,d d d,fff\"", "",
+	                                  NULL };
+
+static const char* testListArgs1[] = { testListAppName, "a", "b", "c", "d" };
+static const char* testListArgs2[] = { testListAppName, "a:", "b:xxx, yyy", "c" };
+// static const char* testListArgs3[] = {};
+// static const char* testListArgs4[] = {};
+static const char* testListArgs5[] = { testListAppName, "a", "b", "c", "d d d", "fff" };
+static const char* testListArgs6[] = { testListAppName };
+static const char* testListArgs7[] = { testListAppName };
+
+static const char** testListArgsResult[] = {
+	testListArgs1, testListArgs2, NULL /* testListArgs3 */, NULL /* testListArgs4 */, testListArgs5,
+	testListArgs6, testListArgs7
+};
+static const size_t testListArgsCount[] = {
+	ARRAYSIZE(testListArgs1),         ARRAYSIZE(testListArgs2), 0 /* ARRAYSIZE(testListArgs3) */,
+	0 /* ARRAYSIZE(testListArgs4) */, ARRAYSIZE(testListArgs5), ARRAYSIZE(testListArgs6),
+	ARRAYSIZE(testListArgs7)
+};
+
+static BOOL checkResult(size_t index, char** actual, size_t actualCount)
+{
+	const char** result = testListArgsResult[index];
+	const size_t resultCount = testListArgsCount[index];
+
+	if (resultCount != actualCount)
+		return FALSE;
+
+	if (actualCount == 0)
+	{
+		return (actual == NULL);
+	}
+	else
+	{
+		size_t x;
+
+		if (!actual)
+			return FALSE;
+
+		for (x = 0; x < actualCount; x++)
+		{
+			const char* a = result[x];
+			const char* b = actual[x];
+
+			if (strcmp(a, b) != 0)
+				return FALSE;
+		}
+	}
+
+	return TRUE;
+}
+
+static BOOL TestCommandLineParseCommaSeparatedValuesEx(void)
+{
+	size_t x;
+
+	WINPR_ASSERT(ARRAYSIZE(testListArgs) == ARRAYSIZE(testListArgsResult));
+	WINPR_ASSERT(ARRAYSIZE(testListArgs) == ARRAYSIZE(testListArgsCount));
+
+	for (x = 0; x < ARRAYSIZE(testListArgs); x++)
+	{
+		const char* list = testListArgs[x];
+		size_t count = 42;
+		char** ptr = CommandLineParseCommaSeparatedValuesEx(testListAppName, list, &count);
+		BOOL valid = checkResult(x, ptr, count);
+		free(ptr);
+		if (!valid)
+			return FALSE;
+	}
+
+	return TRUE;
+}
+
 int TestCmdLine(int argc, char* argv[])
 {
 	int status;
@@ -250,5 +327,8 @@ int TestCmdLine(int argc, char* argv[])
 
 out:
 	string_list_free(command_line);
+
+	if (!TestCommandLineParseCommaSeparatedValuesEx())
+		return -1;
 	return ret;
 }