From b3f01f293a9a03e3ea4de1162cb7f2b4e26a64b0 Mon Sep 17 00:00:00 2001 From: JakobDev Date: Thu, 24 Nov 2022 16:15:07 +0100 Subject: [PATCH 1/2] Improve Screenshot validation --- src/as-validator-issue-tag.h | 40 ++++++++++++++++++++ src/as-validator.c | 73 +++++++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/src/as-validator-issue-tag.h b/src/as-validator-issue-tag.h index 6c9852a5..b6cfa68f 100644 --- a/src/as-validator-issue-tag.h +++ b/src/as-validator-issue-tag.h @@ -204,6 +204,41 @@ AsValidatorIssueTag as_validator_issue_tag_list[] = { N_("The update-contact does not appear to be a valid email address (escaping of `@` is only allowed as `_at_` or `_AT_`).") }, + { "screenshot-invalid-width", + AS_ISSUE_SEVERITY_ERROR, + N_("The width property must be a integer") + }, + + { "screenshot-invalid-height", + AS_ISSUE_SEVERITY_ERROR, + N_("The width property must be a integer") + }, + + { "screenshot-image-invalid-type", + AS_ISSUE_SEVERITY_ERROR, + N_("The type of the image is neither source or thumbnail") + }, + + { "screenshot-image-missing-width", + AS_ISSUE_SEVERITY_ERROR, + N_("The width property must be present if the type is thumbnail") + }, + + { "screenshot-image-missing-height", + AS_ISSUE_SEVERITY_ERROR, + N_("The height property must be present if the type is thumbnail") + }, + + { "screenshot-image-source-duplicated", + AS_ISSUE_SEVERITY_ERROR, + N_("The source type can only be used once for a image and a language") + }, + + { "screenshot-image-source-missing", + AS_ISSUE_SEVERITY_ERROR, + N_("A screenshot must have at leat one image with the source type") + }, + { "screenshot-image-not-found", AS_ISSUE_SEVERITY_WARNING, N_("Unable to reach the screenshot image on its remote location - does the image exist?") @@ -273,6 +308,11 @@ AsValidatorIssueTag as_validator_issue_tag_list[] = { N_("The default screenshot of a software component must not be a video. Use a static image as default screenshot and set the video as a secondary screenshot.") }, + { "screenshot-default-missing", + AS_ISSUE_SEVERITY_ERROR, + N_("There is no defualt screenshot") + }, + { "relation-invalid-tag", AS_ISSUE_SEVERITY_WARNING, N_("Found an unknown tag in a requires/recommends group. This is likely an error, because a component relation of this type is unknown.") diff --git a/src/as-validator.c b/src/as-validator.c index 27de13f0..48359373 100644 --- a/src/as-validator.c +++ b/src/as-validator.c @@ -1228,6 +1228,7 @@ as_validator_check_tags (AsValidator *validator, xmlNode *node) static void as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsComponent *cpt) { + gboolean default_screenshot_found = FALSE; for (xmlNode *iter = node->children; iter != NULL; iter = iter->next) { xmlNode *iter2; gboolean image_found = FALSE; @@ -1235,13 +1236,19 @@ as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsCompone gboolean caption_found = FALSE; gboolean default_screenshot = FALSE; g_autofree gchar *scr_kind_str = NULL; + gboolean default_source_found = FALSE; + g_autoptr(GHashTable) known_source_lang = NULL; + + known_source_lang = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); if (iter->type != XML_ELEMENT_NODE) continue; scr_kind_str = as_xml_get_prop_value (iter, "type"); - if (g_strcmp0 (scr_kind_str, "default") == 0) + if (g_strcmp0 (scr_kind_str, "default") == 0) { + default_screenshot_found = TRUE; default_screenshot = TRUE; + } if (g_strcmp0 ((const gchar*) iter->name, "screenshot") != 0) { as_validator_add_issue (validator, iter, @@ -1257,10 +1264,69 @@ as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsCompone if (iter2->type != XML_ELEMENT_NODE) continue; + g_autofree gchar *screenshot_width = NULL; + g_autofree gchar *screenshot_height = NULL; + + screenshot_width = as_xml_get_prop_value (iter2, "width"); + if (screenshot_width != NULL && !as_str_verify_integer (screenshot_width, 1, G_MAXINT64)) + as_validator_add_issue (validator, iter2, + "screenshot-invalid-width", + screenshot_width); + + screenshot_height = as_xml_get_prop_value (iter2, "height"); + if (screenshot_height != NULL && !as_str_verify_integer (screenshot_height, 1, G_MAXINT64)) + as_validator_add_issue (validator, iter2, + "screenshot-invalid-height", + screenshot_height); + if (g_strcmp0 (node_name, "image") == 0) { g_autofree gchar *image_url = as_strstripnl ((gchar*) xmlNodeGetContent (iter2)); + g_autofree gchar *image_type = NULL; image_found = TRUE; + + image_type = as_xml_get_prop_value (iter2, "type"); + if (image_type == NULL) + image_type = "source"; + + if (g_strcmp0 (image_type, "source") != 0 && g_strcmp0 (image_type, "thumbnail") != 0) + as_validator_add_issue (validator, iter2, + "screenshot-image-invalid-type", + image_type); + + if (screenshot_width == NULL && g_strcmp0 (image_type, "thumbnail") == 0) + as_validator_add_issue (validator, iter2, + "screenshot-image-missing-width", + NULL); + + if (screenshot_height == NULL && g_strcmp0 (image_type, "thumbnail") == 0) + as_validator_add_issue (validator, iter2, + "screenshot-image-missing-height", + NULL); + + if (g_strcmp0(image_type, "source") == 0) { + gchar *image_lang = NULL; + + image_lang = as_xml_get_prop_value (iter2, "lang"); + + if (image_lang == NULL) { + if (default_source_found) + as_validator_add_issue (validator, iter2, + "screenshot-image-source-duplicated", + NULL); + + default_source_found = TRUE; + } + else { + if (g_hash_table_contains (known_source_lang, image_lang)) + as_validator_add_issue (validator, iter2, + "screenshot-image-source-duplicated", + image_lang); + + g_hash_table_add (known_source_lang, image_lang); + } + } + if (!as_validate_is_url (image_url)) { as_validator_add_issue (validator, iter2, "web-url-expected", @@ -1351,10 +1417,15 @@ as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsCompone as_validator_add_issue (validator, iter, "screenshot-no-media", NULL); else if (image_found && video_found) as_validator_add_issue (validator, iter, "screenshot-mixed-images-videos", NULL); + else if (image_found && !default_source_found) + as_validator_add_issue (validator, iter, "screenshot-image-source-missing", NULL); if (!caption_found) as_validator_add_issue (validator, iter, "screenshot-no-caption", NULL); } + + if (!default_screenshot_found) + as_validator_add_issue (validator, node, "screenshot-default-missing", NULL); } /** From 04089113c81f7a2bd12ffe4e39af3223b2e71300 Mon Sep 17 00:00:00 2001 From: JakobDev Date: Thu, 22 Dec 2022 14:20:19 +0100 Subject: [PATCH 2/2] Apply suggestions --- src/as-validator-issue-tag.h | 18 +++++++++--------- src/as-validator.c | 7 ++++--- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/as-validator-issue-tag.h b/src/as-validator-issue-tag.h index b6cfa68f..7c044896 100644 --- a/src/as-validator-issue-tag.h +++ b/src/as-validator-issue-tag.h @@ -206,7 +206,7 @@ AsValidatorIssueTag as_validator_issue_tag_list[] = { { "screenshot-invalid-width", AS_ISSUE_SEVERITY_ERROR, - N_("The width property must be a integer") + N_("The width property must be a positive integer") }, { "screenshot-invalid-height", @@ -216,27 +216,27 @@ AsValidatorIssueTag as_validator_issue_tag_list[] = { { "screenshot-image-invalid-type", AS_ISSUE_SEVERITY_ERROR, - N_("The type of the image is neither source or thumbnail") + N_("The type of the image is neither `source` or `thumbnail`") }, { "screenshot-image-missing-width", - AS_ISSUE_SEVERITY_ERROR, - N_("The width property must be present if the type is thumbnail") + AS_ISSUE_SEVERITY_WARNING, + N_("The `width` property must be present if the type is `thumbnail`") }, { "screenshot-image-missing-height", - AS_ISSUE_SEVERITY_ERROR, - N_("The height property must be present if the type is thumbnail") + AS_ISSUE_SEVERITY_WARNING, + N_("The `height` property must be present if the type is thumbnail") }, { "screenshot-image-source-duplicated", AS_ISSUE_SEVERITY_ERROR, - N_("The source type can only be used once for a image and a language") + N_("The 'source' type can only be used once for a image and a language") }, { "screenshot-image-source-missing", AS_ISSUE_SEVERITY_ERROR, - N_("A screenshot must have at leat one image with the source type") + N_("A screenshot must have at least one image of type `source`") }, { "screenshot-image-not-found", @@ -310,7 +310,7 @@ AsValidatorIssueTag as_validator_issue_tag_list[] = { { "screenshot-default-missing", AS_ISSUE_SEVERITY_ERROR, - N_("There is no defualt screenshot") + N_("A default screenshot is missing.") }, { "relation-invalid-tag", diff --git a/src/as-validator.c b/src/as-validator.c index 48359373..9f73cd5a 100644 --- a/src/as-validator.c +++ b/src/as-validator.c @@ -1245,7 +1245,7 @@ as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsCompone continue; scr_kind_str = as_xml_get_prop_value (iter, "type"); - if (g_strcmp0 (scr_kind_str, "default") == 0) { + if (as_screenshot_kind_from_string (scr_kind_str) == AS_SCREENSHOT_KIND_DEFAULT) { default_screenshot_found = TRUE; default_screenshot = TRUE; } @@ -1260,12 +1260,13 @@ as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsCompone } for (iter2 = iter->children; iter2 != NULL; iter2 = iter2->next) { + g_autofree gchar *screenshot_width = NULL; + g_autofree gchar *screenshot_height = NULL; + const gchar *node_name = (const gchar*) iter2->name; if (iter2->type != XML_ELEMENT_NODE) continue; - g_autofree gchar *screenshot_width = NULL; - g_autofree gchar *screenshot_height = NULL; screenshot_width = as_xml_get_prop_value (iter2, "width"); if (screenshot_width != NULL && !as_str_verify_integer (screenshot_width, 1, G_MAXINT64))