From cf73b762112730acfd4f7b11594a30a2b49e2443 Mon Sep 17 00:00:00 2001
From: Otto Urpelainen <oturpe@iki.fi>
Date: Thu, 13 May 2021 23:21:34 +0300
Subject: Add validation for duplicate license xml elements

AppStream license syntax allows constructs of form "LICENSE1 AND
LICENSE2". It is possible to confuse this with writing multiple license
xml elements in a metainfo file. In order to spot this type of error,
duplicate detection is added for xml elements metadata_license and
project_license.
---
 data/tests/broken.appdata.xml       |  1 +
 libappstream-glib/as-app-private.h  |  2 ++
 libappstream-glib/as-app-validate.c | 10 ++++++++++
 libappstream-glib/as-app.c          |  6 +++++-
 libappstream-glib/as-self-test.c    |  8 ++++++--
 5 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/data/tests/broken.appdata.xml b/data/tests/broken.appdata.xml
index 1603848..e4c3148 100644
--- a/data/tests/broken.appdata.xml
+++ b/data/tests/broken.appdata.xml
@@ -4,6 +4,7 @@
   <metadata_license>GPLv3+</metadata_license>
   <metadata_license>BSD</metadata_license>
   <project_license>CC1</project_license>
+  <project_license>CC1</project_license>
   <project_group>GNU</project_group>
   <name>Power Statistics.</name>
   <summary>Observe power management.</summary>
diff --git a/libappstream-glib/as-app-private.h b/libappstream-glib/as-app-private.h
index 5089ba8..9c99236 100644
--- a/libappstream-glib/as-app-private.h
+++ b/libappstream-glib/as-app-private.h
@@ -69,6 +69,8 @@ typedef enum {
 	AS_APP_PROBLEM_DUPLICATE_SCREENSHOT	= 1 << 18,
 	AS_APP_PROBLEM_DUPLICATE_CONTENT_RATING	= 1 << 19,
 	AS_APP_PROBLEM_DUPLICATE_AGREEMENT	= 1 << 20,
+	AS_APP_PROBLEM_DUPLICATE_PROJECT_LICENSE = 1 << 21,
+	AS_APP_PROBLEM_DUPLICATE_METADATA_LICENSE = 1 << 22,
 	/*< private >*/
 	AS_APP_PROBLEM_LAST
 } AsAppProblems;
diff --git a/libappstream-glib/as-app-validate.c b/libappstream-glib/as-app-validate.c
index 60b46e5..7de7814 100644
--- a/libappstream-glib/as-app-validate.c
+++ b/libappstream-glib/as-app-validate.c
@@ -1525,6 +1525,16 @@ as_app_validate (AsApp *app, guint32 flags, GError **error)
 					     AS_PROBLEM_KIND_TAG_INVALID,
 					     "<updatecontact> should be <update_contact>");
 		}
+		if ((problems & AS_APP_PROBLEM_DUPLICATE_PROJECT_LICENSE) > 0) {
+			ai_app_validate_add(helper,
+					    AS_PROBLEM_KIND_TAG_INVALID,
+					    "<project_license> was duplicated");
+		}
+		if ((problems & AS_APP_PROBLEM_DUPLICATE_METADATA_LICENSE) > 0) {
+			ai_app_validate_add(helper,
+					    AS_PROBLEM_KIND_TAG_INVALID,
+					    "<metadata_license> was duplicated");
+		}
 	}
 
 	/* check invalid values */
diff --git a/libappstream-glib/as-app.c b/libappstream-glib/as-app.c
index 2caba19..3939970 100644
--- a/libappstream-glib/as-app.c
+++ b/libappstream-glib/as-app.c
@@ -5154,15 +5154,19 @@ as_app_node_parse_child (AsApp *app, GNode *n, guint32 flags,
 			priv->problems |= AS_APP_PROBLEM_TRANSLATED_LICENSE;
 			break;
 		}
+		if (priv->project_license != NULL)
+			priv->problems |= AS_APP_PROBLEM_DUPLICATE_PROJECT_LICENSE;
 		as_ref_string_assign (&priv->project_license, as_node_get_data_as_refstr (n));
 		break;
 
-	/* <project_license> */
+	/* <metadata_license> */
 	case AS_TAG_METADATA_LICENSE:
 		if (as_node_get_attribute (n, "xml:lang") != NULL) {
 			priv->problems |= AS_APP_PROBLEM_TRANSLATED_LICENSE;
 			break;
 		}
+		if (priv->metadata_license != NULL)
+			priv->problems |= AS_APP_PROBLEM_DUPLICATE_METADATA_LICENSE;
 		as_app_set_metadata_license (app, as_node_get_data (n));
 		break;
 
diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c
index 29fc20a..2f0820f 100644
--- a/libappstream-glib/as-self-test.c
+++ b/libappstream-glib/as-self-test.c
@@ -2394,8 +2394,12 @@ as_test_app_validate_file_bad_func (void)
 				    "<component> has invalid type attribute");
 	as_test_app_validate_check (probs, AS_PROBLEM_KIND_TAG_INVALID,
 				    "<metadata_license> is not valid");
+	as_test_app_validate_check (probs, AS_PROBLEM_KIND_TAG_INVALID,
+				    "<metadata_license> was duplicated");
 	as_test_app_validate_check (probs, AS_PROBLEM_KIND_TAG_INVALID,
 				    "<project_license> is not valid");
+	as_test_app_validate_check (probs, AS_PROBLEM_KIND_TAG_INVALID,
+				    "<project_license> was duplicated");
 	as_test_app_validate_check (probs, AS_PROBLEM_KIND_TAG_INVALID,
 				    "<url> does not start with 'http://'");
 	as_test_app_validate_check (probs, AS_PROBLEM_KIND_MARKUP_INVALID,
@@ -2437,7 +2441,7 @@ as_test_app_validate_file_bad_func (void)
 				    "<launchable> has invalid type attribute");
 	as_test_app_validate_check (probs, AS_PROBLEM_KIND_VALUE_MISSING,
 				    "<launchable> missing value");
-	g_assert_cmpint (probs->len, ==, 23);
+	g_assert_cmpint (probs->len, ==, 25);
 
 	/* again, harder */
 	probs2 = as_app_validate (app, AS_APP_VALIDATE_FLAG_STRICT, &error);
@@ -2445,7 +2449,7 @@ as_test_app_validate_file_bad_func (void)
 	g_assert (probs2 != NULL);
 	as_test_app_validate_check (probs2, AS_PROBLEM_KIND_TAG_INVALID,
 				    "XML data contains unknown tag");
-	g_assert_cmpint (probs2->len, ==, 37);
+	g_assert_cmpint (probs2->len, ==, 39);
 }
 
 static void
-- 
cgit v1.2.1