Return proper warnings when using libyaml

Works around https://bugs.launchpad.net/ubuntu/+source/fwupd/+bug/1591868
although the real bug lies in whatever project wrote that invalid DEP-11 file.

2 files changed, 192 insertions, 15 deletions

diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c
index 35c94d0..5681766 100644
--- a/libappstream-glib/as-self-test.c
+++ b/libappstream-glib/as-self-test.c
@@ -4115,6 +4115,31 @@ as_test_store_metadata_index_func (void)
 }
 
 static void
+as_test_yaml_broken_func (void)
+{
+#if AS_BUILD_DEP11
+	g_autoptr(AsYaml) node = NULL;
+	g_autoptr(GError) error1 = NULL;
+	g_autoptr(GError) error2 = NULL;
+	node = as_yaml_from_data ("s---\n"
+				  "File: DEP-11\n",
+				  -1, &error1);
+	g_assert_error (error1, AS_NODE_ERROR, AS_NODE_ERROR_INVALID_MARKUP);
+	g_assert (node == NULL);
+	node = as_yaml_from_data ("---\n"
+				  "%File: DEP-11\n",
+				  -1, &error2);
+	g_assert_error (error2, AS_NODE_ERROR, AS_NODE_ERROR_INVALID_MARKUP);
+	g_assert_cmpstr (error2->message, ==,
+			 "scanner error: while scanning a directive at ln:2 col:1, "
+			 "found unexpected non-alphabetical character at ln:2 col:6");
+	g_assert (node == NULL);
+#else
+	g_test_skip ("Compiled without YAML (DEP-11) support");
+#endif
+}
+
+static void
 as_test_yaml_func (void)
 {
 #if AS_BUILD_DEP11
@@ -4148,6 +4173,7 @@ as_test_yaml_func (void)
 
 	/* simple list */
 	node = as_yaml_from_data (
+		"---\n"
 		"Mimetypes:\n"
 		"  - text/html\n"
 		"  - text/xml\n"
@@ -4915,6 +4941,7 @@ main (int argc, char **argv)
 		g_test_add_func ("/AppStream/monitor{file}", as_test_monitor_file_func);
 	}
 	g_test_add_func ("/AppStream/yaml", as_test_yaml_func);
+	g_test_add_func ("/AppStream/yaml{broken}", as_test_yaml_broken_func);
 	g_test_add_func ("/AppStream/store", as_test_store_func);
 	g_test_add_func ("/AppStream/store{empty}", as_test_store_empty_func);
 	if (g_test_slow ()) {
diff --git a/libappstream-glib/as-yaml.c b/libappstream-glib/as-yaml.c
index 8aed749..f65f75b 100644
--- a/libappstream-glib/as-yaml.c
+++ b/libappstream-glib/as-yaml.c
@@ -181,8 +181,122 @@ as_yaml_node_new (AsYamlNodeKind kind, const gchar *id)
 	return ym;
 }
 
-static void
-as_node_yaml_process_layer (yaml_parser_t *parser, AsNode *parent)
+static gchar *
+as_yaml_mark_to_str (yaml_mark_t *mark)
+{
+	return g_strdup_printf ("ln:%" G_GSIZE_FORMAT " col:%" G_GSIZE_FORMAT,
+				mark->line + 1,
+				mark->column + 1);
+}
+
+static gboolean
+as_yaml_parser_error_to_gerror (yaml_parser_t *parser, GError **error)
+{
+	g_autofree gchar *problem_str = NULL;
+	g_autofree gchar *context_str = NULL;
+
+	switch (parser->error) {
+	case YAML_MEMORY_ERROR:
+		g_set_error_literal (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "not enough memory for parsing");
+		break;
+
+	case YAML_READER_ERROR:
+		if (parser->problem_value != -1) {
+			g_set_error (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "reader error: %s: #%X at %" G_GSIZE_FORMAT "",
+				     parser->problem,
+				     (guint) parser->problem_value,
+				     parser->problem_offset);
+		} else {
+			g_set_error (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "reader error: %s at %" G_GSIZE_FORMAT "",
+				     parser->problem,
+				     parser->problem_offset);
+		}
+		break;
+
+	case YAML_SCANNER_ERROR:
+		problem_str = as_yaml_mark_to_str (&parser->problem_mark);
+		if (parser->context) {
+			context_str = as_yaml_mark_to_str (&parser->context_mark);
+			g_set_error (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "scanner error: %s at %s, %s at %s",
+				     parser->context,
+				     context_str,
+				     parser->problem,
+				     problem_str);
+		} else {
+			g_set_error (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "scanner error: %s at %s ",
+				     parser->problem,
+				     problem_str);
+		}
+		break;
+	case YAML_PARSER_ERROR:
+		problem_str = as_yaml_mark_to_str (&parser->problem_mark);
+		if (parser->context) {
+			context_str = as_yaml_mark_to_str (&parser->context_mark);
+			g_set_error (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "parser error: %s at %s, %s at %s",
+				     parser->context,
+				     context_str,
+				     parser->problem,
+				     problem_str);
+		} else {
+			g_set_error (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "parser error: %s at %s ",
+				     parser->problem,
+				     problem_str);
+		}
+		break;
+	case YAML_COMPOSER_ERROR:
+		problem_str = as_yaml_mark_to_str (&parser->problem_mark);
+		if (parser->context) {
+			context_str = as_yaml_mark_to_str (&parser->context_mark);
+			g_set_error (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "composer error: %s at %s, %s at %s",
+				     parser->context,
+				     context_str,
+				     parser->problem,
+				     problem_str);
+		} else {
+			g_set_error (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "composer error: %s at %s ",
+				     parser->problem, problem_str);
+		}
+		break;
+	default:
+		/* can't happen */
+		g_set_error_literal (error,
+				     AS_NODE_ERROR,
+				     AS_NODE_ERROR_INVALID_MARKUP,
+				     "internal error");
+		break;
+	}
+	return FALSE;
+}
+
+static gboolean
+as_node_yaml_process_layer (yaml_parser_t *parser, AsNode *parent, GError **error)
 {
 	AsYamlNode *ym;
 	AsNode *last_scalar = NULL;
@@ -192,7 +306,11 @@ as_node_yaml_process_layer (yaml_parser_t *parser, AsNode *parent)
 	yaml_event_t event;
 
 	while (valid) {
-		yaml_parser_parse (parser, &event);
+
+		/* process event */
+		if (!yaml_parser_parse (parser, &event))
+			return as_yaml_parser_error_to_gerror (parser, error);
+
 		switch (event.type) {
 		case YAML_SCALAR_EVENT:
 			tmp = (const gchar *) event.data.scalar.value;
@@ -216,7 +334,8 @@ as_node_yaml_process_layer (yaml_parser_t *parser, AsNode *parent)
 				ym->kind = AS_YAML_NODE_KIND_MAP;
 				new = last_scalar;
 			}
-			as_node_yaml_process_layer (parser, new);
+			if (!as_node_yaml_process_layer (parser, new, error))
+				return FALSE;
 			last_scalar = NULL;
 			break;
 		case YAML_SEQUENCE_START_EVENT:
@@ -228,7 +347,8 @@ as_node_yaml_process_layer (yaml_parser_t *parser, AsNode *parent)
 				ym->kind = AS_YAML_NODE_KIND_SEQ;
 				new = last_scalar;
 			}
-			as_node_yaml_process_layer (parser, new);
+			if (!as_node_yaml_process_layer (parser, new, error))
+				return FALSE;
 			last_scalar = NULL;
 			break;
 		case YAML_STREAM_START_EVENT:
@@ -243,31 +363,56 @@ as_node_yaml_process_layer (yaml_parser_t *parser, AsNode *parent)
 		}
 		yaml_event_delete (&event);
 	}
+	return TRUE;
 }
+
+typedef yaml_parser_t* AsYamlParser;
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(AsYamlParser, yaml_parser_delete, NULL);
+
 #endif
 
 AsNode *
 as_yaml_from_data (const gchar *data, gssize data_len, GError **error)
 {
-	AsNode *node = NULL;
+	g_autoptr(AsNode) node = NULL;
 #if AS_BUILD_DEP11
 	yaml_parser_t parser;
+	g_auto(AsYamlParser) parser_cleanup = NULL;
+	g_autofree gchar *prefix = NULL;
+
+	/* sanity check */
+	prefix = g_strndup (data, 64);
+	g_strdelimit  (prefix, "\n", '\0');
+	if (!g_str_has_prefix (prefix, "---") &&
+	    !g_str_has_prefix (prefix, "#") &&
+	    !g_str_has_prefix (prefix, "File: ")) {
+		g_set_error (error,
+			     AS_NODE_ERROR,
+			     AS_NODE_ERROR_INVALID_MARKUP,
+			     "YAML prefix invalid: %s expected '---' or '#'",
+			     prefix);
+		return NULL;
+	}
 
 	/* parse */
-	yaml_parser_initialize (&parser);
+	if (!yaml_parser_initialize (&parser)) {
+		as_yaml_parser_error_to_gerror (&parser, error);
+		return NULL;
+	}
+	parser_cleanup = &parser;
 	if (data_len < 0)
 		data_len = strlen (data);
 	yaml_parser_set_input_string (&parser, (guchar *) data, data_len);
 	node = g_node_new (NULL);
-	as_node_yaml_process_layer (&parser, node);
-	yaml_parser_delete (&parser);
+	if (!as_node_yaml_process_layer (&parser, node, error))
+		return NULL;
 #else
 	g_set_error_literal (error,
 			     AS_NODE_ERROR,
 			     AS_NODE_ERROR_NO_SUPPORT,
 			     "No DEP-11 support, needs libyaml");
 #endif
-	return node;
+	return g_steal_pointer (&node);
 }
 
 #if AS_BUILD_DEP11
@@ -286,10 +431,11 @@ as_yaml_read_handler_cb (void *data,
 AsNode *
 as_yaml_from_file (GFile *file, GCancellable *cancellable, GError **error)
 {
-	AsNode *node = NULL;
+	g_autoptr(AsNode) node = NULL;
 #if AS_BUILD_DEP11
 	const gchar *content_type = NULL;
 	yaml_parser_t parser;
+	g_auto(AsYamlParser) parser_cleanup = NULL;
 	g_autofree gchar *data = NULL;
 	g_autoptr(GConverter) conv = NULL;
 	g_autoptr(GFileInfo) info = NULL;
@@ -326,16 +472,20 @@ as_yaml_from_file (GFile *file, GCancellable *cancellable, GError **error)
 	}
 
 	/* parse */
-	yaml_parser_initialize (&parser);
+	if (!yaml_parser_initialize (&parser)) {
+		as_yaml_parser_error_to_gerror (&parser, error);
+		return NULL;
+	}
+	parser_cleanup = &parser;
 	yaml_parser_set_input (&parser, as_yaml_read_handler_cb, stream_data);
 	node = g_node_new (NULL);
-	as_node_yaml_process_layer (&parser, node);
-	yaml_parser_delete (&parser);
+	if (!as_node_yaml_process_layer (&parser, node, error))
+		return NULL;
 #else
 	g_set_error_literal (error,
 			     AS_NODE_ERROR,
 			     AS_NODE_ERROR_NO_SUPPORT,
 			     "No DEP-11 support, needs libyaml");
 #endif
-	return node;
+	return g_steal_pointer (&node);
 }