registry: Restore default libxml2 error handler after parsing

Leaving the custom error handler could have resulted in a crash after
the context has been freed.

Closes: #529

Author: sbstnk

changes/api/529.fix-xml-error-handler.bugfix.md

@@ -0,0 +1,4 @@
+registry: Fixed `libxml2` global error handler not reset after parsing, which
+could trigger a crash if the corresponding `rxkb_context` has been freed.
+
+Contributed by Sebastian Keller.

meson.build

@@ -879,7 +879,7 @@ if get_option('enable-xkbregistry')
         'registry',
         executable('test-registry', 'test/registry.c',
                    include_directories: include_directories('src'),
-                   dependencies: [dep_libxkbregistry, test_dep]),
+                   dependencies: [dep_libxkbregistry, dep_libxml, test_dep]),
         env: test_env,
     )
 endif

src/registry.c

@@ -1250,20 +1250,32 @@ parse(struct rxkb_context *ctx, const char *path,
 
     doc = xmlParseFile(path);
     if (!doc)
-        return false;
+        goto parse_error;
 
     if (!validate(ctx, doc)) {
         log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                 "XML error: failed to validate document at %s\n", path);
-        goto error;
+        goto validate_error;
     }
 
     root = xmlDocGetRootElement(doc);
     parse_rules_xml(ctx, root, popularity);
 
     success = true;
-error:
+validate_error:
     xmlFreeDoc(doc);
+parse_error:
+    /*
+     * Reset the default libxml2 error handler to default, because this handler
+     * is global and may be used on an invalid rxkb_context, e.g. *after* the
+     * following sequence:
+     *
+     *     rxkb_context_new();
+     *     rxkb_context_parse();
+     *     rxkb_context_unref();
+     */
+    /* TODO: use the new API xmlCtxtSetErrorHandler */
+    xmlSetGenericErrorFunc(NULL, NULL);
 
     return success;
 }

test/registry.c

@@ -31,6 +31,8 @@
 #include <stdio.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <libxml/parser.h>
+#include <libxml/tree.h>
 
 #include "xkbcommon/xkbregistry.h"
 
@@ -1065,11 +1067,36 @@ test_invalid_include(void)
     rxkb_context_unref(ctx);
 }
 
+/* Check that libxml2 error handler is reset after parsing */
+static void
+test_xml_error_handler(void)
+{
+    struct test_model system_models[] =  { {NULL} };
+    struct test_layout system_layouts[] =  { {NULL} };
+    struct test_option_group system_groups[] = { { NULL } };
+    struct rxkb_context *ctx;
+
+    ctx = test_setup_context(system_models, NULL,
+                             system_layouts, NULL,
+                             system_groups, NULL);
+    assert(ctx);
+    rxkb_context_unref(ctx);
+
+    const char invalid_xml[] = "<test";
+    /* This should trigger a segfault if error handler is not reset, because
+     * else it would use our error handler with an invalid (freed) context. */
+    xmlDocPtr doc = xmlParseMemory (invalid_xml, strlen(invalid_xml));
+    assert(!doc);
+    xmlFreeDoc (doc);
+    xmlCleanupParser();
+}
+
 int
 main(void)
 {
     test_init();
 
+    test_xml_error_handler();
     test_no_include_paths();
     test_invalid_include();
     test_load_basic();