[PATCH 1/2] stk: Constraint the appearance of empty item

[PATCH 1/2] stk: Constraint the appearance of empty item

[Yang Gu]

[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]

If empty item turns out, it must be the only item in the list.
So the following situations are deemed as invalid:
1. Two empty items
2. Valid item + empty item
3. empty + valid item
---
 src/stkutil.c |   40 ++++++++++++++++++++++++++++------------
 1 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 2ca5c6a..a31ae20 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -2014,6 +2014,12 @@ static dataobj_handler handler_for_type(enum stk_data_object_type type)
 	}
 }
 
+static void destroy_stk_item(struct stk_item *item)
+{
+	g_free(item->text);
+	g_free(item);
+}
+
 static gboolean parse_item_list(struct comprehension_tlv_iter *iter,
 				void *data)
 {
@@ -2022,21 +2028,40 @@ static gboolean parse_item_list(struct comprehension_tlv_iter *iter,
 	struct comprehension_tlv_iter iter_old;
 	struct stk_item item;
 	GSList *list = NULL;
+	unsigned int count = 0;
+	gboolean has_empty = FALSE;
 
 	do {
 		comprehension_tlv_iter_copy(iter, &iter_old);
 		memset(&item, 0, sizeof(item));
+		count++;
+
+		if (parse_dataobj_item(iter, &item) == TRUE) {
+			if (item.id == 0) {
+				has_empty = TRUE;
+				continue;
+			}
 
-		if (parse_dataobj_item(iter, &item) == TRUE)
 			list = g_slist_prepend(list,
 						g_memdup(&item, sizeof(item)));
+		}
 	} while (comprehension_tlv_iter_next(iter) == TRUE &&
 			comprehension_tlv_iter_get_tag(iter) == tag);
 
 	comprehension_tlv_iter_copy(&iter_old, iter);
-	*out = g_slist_reverse(list);
 
-	return TRUE;
+	if (!has_empty) {
+		*out = g_slist_reverse(list);
+		return TRUE;
+	}
+
+	if (count == 1)
+		return TRUE;
+
+	g_slist_foreach(list, (GFunc)destroy_stk_item, NULL);
+	g_slist_free(list);
+	return FALSE;
+
 }
 
 static gboolean parse_provisioning_list(struct comprehension_tlv_iter *iter,
@@ -2342,12 +2367,6 @@ static gboolean parse_poll_interval(struct stk_command *command,
 	return TRUE;
 }
 
-static void destroy_stk_item(struct stk_item *item)
-{
-	g_free(item->text);
-	g_free(item);
-}
-
 static void destroy_setup_menu(struct stk_command *command)
 {
 	g_free(command->setup_menu.alpha_id);
@@ -2392,9 +2411,6 @@ static gboolean parse_setup_menu(struct stk_command *command,
 	if (ret == FALSE)
 		return FALSE;
 
-	if (obj->items == NULL)
-		return FALSE;
-
 	return TRUE;
 }
 
-- 
1.7.0.4

[PATCH 2/2] stktest: Add negative cases for setup menu

[Yang Gu]

[-- Attachment #1: Type: text/plain, Size: 3453 bytes --]

---
 unit/test-stkutil.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
index 7793f3a..b4a5618 100644
--- a/unit/test-stkutil.c
+++ b/unit/test-stkutil.c
@@ -4893,6 +4893,31 @@ static unsigned char setup_menu_913[] = { 0xD0, 0x0D, 0x81, 0x03, 0x01, 0x25,
 						0x00, 0x82, 0x02, 0x81, 0x82,
 						0x85, 0x00, 0x8F, 0x00 };
 
+/* Negative case: No item is present */
+static unsigned char setup_menu_neg_1[] = { 0xD0, 0x0B, 0x81, 0x03, 0x01, 0x25,
+						0x00, 0x82, 0x02, 0x81, 0x82,
+						0x85, 0x00 };
+
+/* Negative case: Two empty items*/
+static unsigned char setup_menu_neg_2[] = { 0xD0, 0x0F, 0x81, 0x03, 0x01, 0x25,
+						0x00, 0x82, 0x02, 0x81, 0x82,
+						0x85, 0x00, 0x8F, 0x00, 0x8F,
+						0x00 };
+
+/* Negative case: valid item + empty item */
+static unsigned char setup_menu_neg_3[] = { 0xD0, 0x16, 0x81, 0x03, 0x01, 0x25,
+						0x00, 0x82, 0x02, 0x81, 0x82,
+						0x85, 0x00, 0x8F, 0x07, 0x01,
+						0x49, 0x74, 0x65, 0x6D, 0x20,
+						0x31, 0x8F, 0x00 };
+
+/* Negative case: empty item + valid item */
+static unsigned char setup_menu_neg_4[] = { 0xD0, 0x16, 0x81, 0x03, 0x01, 0x25,
+						0x00, 0x82, 0x02, 0x81, 0x82,
+						0x85, 0x00, 0x8F, 0x00, 0x8F,
+						0x07, 0x01, 0x49, 0x74, 0x65,
+						0x6D, 0x20, 0x31 };
+
 static struct setup_menu_test setup_menu_data_111 = {
 	.pdu = setup_menu_111,
 	.pdu_len = sizeof(setup_menu_111),
@@ -5451,6 +5476,26 @@ static struct setup_menu_test setup_menu_data_913 = {
 	.qualifier = 0x00
 };
 
+static struct setup_menu_test setup_menu_data_neg_1 = {
+	.pdu = setup_menu_neg_1,
+	.pdu_len = sizeof(setup_menu_neg_1)
+};
+
+static struct setup_menu_test setup_menu_data_neg_2 = {
+	.pdu = setup_menu_neg_2,
+	.pdu_len = sizeof(setup_menu_neg_2)
+};
+
+static struct setup_menu_test setup_menu_data_neg_3 = {
+	.pdu = setup_menu_neg_3,
+	.pdu_len = sizeof(setup_menu_neg_3)
+};
+
+static struct setup_menu_test setup_menu_data_neg_4 = {
+	.pdu = setup_menu_neg_4,
+	.pdu_len = sizeof(setup_menu_neg_4)
+};
+
 /* Defined in TS 102.384 Section 27.22.4.7 */
 static void test_setup_menu(gconstpointer data)
 {
@@ -5484,6 +5529,16 @@ static void test_setup_menu(gconstpointer data)
 	stk_command_free(command);
 }
 
+static void test_setup_menu_neg(gconstpointer data)
+{
+	const struct setup_menu_test *test = data;
+	struct stk_command *command;
+
+	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
+
+	g_assert(!command);
+}
+
 struct select_item_test {
 	const unsigned char *pdu;
 	unsigned int pdu_len;
@@ -16558,6 +16613,15 @@ int main(int argc, char **argv)
 	g_test_add_data_func("/teststk/Setup Menu 9.1.3",
 				&setup_menu_data_913, test_setup_menu);
 
+	g_test_add_data_func("/teststk/Setup Menu Negative 1",
+			&setup_menu_data_neg_1, test_setup_menu_neg);
+	g_test_add_data_func("/teststk/Setup Menu Negative 2",
+			&setup_menu_data_neg_2, test_setup_menu_neg);
+	g_test_add_data_func("/teststk/Setup Menu Negative 3",
+			&setup_menu_data_neg_3, test_setup_menu_neg);
+	g_test_add_data_func("/teststk/Setup Menu Negative 4",
+			&setup_menu_data_neg_4, test_setup_menu_neg);
+
 	g_test_add_data_func("/teststk/Set Up Menu response 1.1.1",
 				&set_up_menu_response_data_111,
 				test_terminal_response_encoding);
-- 
1.7.0.4

Re: [PATCH 1/2] stk: Constraint the appearance of empty item

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

Hi Yang,

> If empty item turns out, it must be the only item in the list.
> So the following situations are deemed as invalid:
> 1. Two empty items
> 2. Valid item + empty item
> 3. empty + valid item

Both have been applied, thanks.

Regards,
-Denis