diff --git a/modulemd/v2/include/modulemd-2.0/modulemd-module-stream.h b/modulemd/v2/include/modulemd-2.0/modulemd-module-stream.h index c9e8af958..8e9df0507 100644 --- a/modulemd/v2/include/modulemd-2.0/modulemd-module-stream.h +++ b/modulemd/v2/include/modulemd-2.0/modulemd-module-stream.h @@ -107,14 +107,14 @@ modulemd_module_stream_new (guint64 mdversion, * modulemd_module_stream_read_file: * @path: (in): The path to a YAML document containing a module stream * definition. + * @strict: (in): Whether the parser should return failure if it encounters an + * unknown mapping key or if it should ignore it. * @module_name: (in) (nullable): An optional module name to override the * document on disk. Mostly useful in cases where the name is being * auto-detected from git. * @module_stream: (in) (nullable): An optional module stream name to override * the document on disk. Mostly useful in cases where the name is being * auto-detected from git. - * @strict: (in): Whether the parser should return failure if it encounters an - * unknown mapping key or if it should ignore it. * @error: (out): A #GError that will return the reason for a failed read. * * Create a #ModulemdModuleStream object from a YAML file. @@ -139,14 +139,14 @@ modulemd_module_stream_read_file (const gchar *path, * modulemd_module_stream_read_string: * @yaml_string: (in): A YAML document string containing a module stream * definition. + * @strict: (in): Whether the parser should return failure if it encounters an + * unknown mapping key or if it should ignore it. * @module_name: (in) (nullable): An optional module name to override the * document on disk. Mostly useful in cases where the name is being * auto-detected from git. * @module_stream: (in) (nullable): An optional module stream name to override * the document on disk. Mostly useful in cases where the name is being * auto-detected from git. - * @strict: (in): Whether the parser should return failure if it encounters an - * unknown mapping key or if it should ignore it. * @error: (out): A #GError that will return the reason for a failed read. * * Create a #ModulemdModuleStream object from a YAML string. @@ -171,14 +171,14 @@ modulemd_module_stream_read_string (const gchar *yaml_string, * modulemd_module_stream_read_stream: (skip) * @stream: (in): A YAML document as a FILE * containing a module stream * definition. + * @strict: (in): Whether the parser should return failure if it encounters an + * unknown mapping key or if it should ignore it. * @module_name: (in) (nullable): An optional module name to override the * document on disk. Mostly useful in cases where the name is being * auto-detected from git. * @module_stream: (in) (nullable): An optional module stream name to override * the document on disk. Mostly useful in cases where the name is being * auto-detected from git. - * @strict: (in): Whether the parser should return failure if it encounters an - * unknown mapping key or if it should ignore it. * @error: (out): A #GError that will return the reason for a failed read. * * Create a #ModulemdModuleStream object from a YAML file. diff --git a/modulemd/v2/modulemd-module-stream-v1.c b/modulemd/v2/modulemd-module-stream-v1.c index 74c4b9021..3bc2fde06 100644 --- a/modulemd/v2/modulemd-module-stream-v1.c +++ b/modulemd/v2/modulemd-module-stream-v1.c @@ -861,7 +861,7 @@ modulemd_module_stream_v1_set_xmd (ModulemdModuleStreamV1 *self, GVariant *xmd) g_return_if_fail (MODULEMD_IS_MODULE_STREAM_V1 (self)); g_clear_pointer (&self->xmd, g_variant_unref); - self->xmd = xmd; + self->xmd = modulemd_variant_deep_copy (xmd); } GVariant * @@ -1069,7 +1069,7 @@ modulemd_module_stream_v1_copy (ModulemdModuleStream *self, copy, v1_self, servicelevels, modulemd_module_stream_v1_add_servicelevel); if (v1_self->xmd != NULL) - modulemd_module_stream_v1_set_xmd (copy, g_variant_ref (v1_self->xmd)); + modulemd_module_stream_v1_set_xmd (copy, v1_self->xmd); return MODULEMD_MODULE_STREAM (g_steal_pointer (©)); } @@ -1421,7 +1421,7 @@ modulemd_module_stream_v1_parse_yaml (ModulemdSubdocumentInfo *subdoc, return NULL; } modulemd_module_stream_v1_set_xmd (modulestream, xmd); - xmd = NULL; + g_clear_pointer (&xmd, g_variant_unref); } /* Dependencies */ diff --git a/modulemd/v2/modulemd-util.c b/modulemd/v2/modulemd-util.c index 1debe440f..7c2fc1807 100644 --- a/modulemd/v2/modulemd-util.c +++ b/modulemd/v2/modulemd-util.c @@ -271,15 +271,25 @@ modulemd_hash_table_unref (void *table) g_hash_table_unref ((GHashTable *)table); } + +static void +destroy_variant_data (gpointer data) +{ + g_free (data); +} + + GVariant * modulemd_variant_deep_copy (GVariant *variant) { const GVariantType *data_type = g_variant_get_type (variant); gsize data_size = g_variant_get_size (variant); - gconstpointer data = g_variant_get_data (variant); + gpointer data = g_malloc0 (data_size); + + g_variant_store (variant, data); - return g_variant_ref_sink ( - g_variant_new_from_data (data_type, data, data_size, TRUE, NULL, NULL)); + return g_variant_ref_sink (g_variant_new_from_data ( + data_type, data, data_size, FALSE, destroy_variant_data, data)); } diff --git a/modulemd/v2/modulemd-yaml-util.c b/modulemd/v2/modulemd-yaml-util.c index 08dec2f08..1a8402a1e 100644 --- a/modulemd/v2/modulemd-yaml-util.c +++ b/modulemd/v2/modulemd-yaml-util.c @@ -957,7 +957,8 @@ modulemd_yaml_emit_variant (yaml_emitter_t *emitter, g_set_error (error, MODULEMD_YAML_ERROR, MODULEMD_YAML_ERROR_EMIT, - "Unhandled variant type"); + "Unhandled variant type: \"%s\"", + g_variant_get_type_string (variant)); return FALSE; } return TRUE; diff --git a/modulemd/v2/tests/ModulemdTests/modulestream.py b/modulemd/v2/tests/ModulemdTests/modulestream.py index 1c8392cb7..3376e9177 100644 --- a/modulemd/v2/tests/ModulemdTests/modulestream.py +++ b/modulemd/v2/tests/ModulemdTests/modulestream.py @@ -528,10 +528,10 @@ def test_xmd(self): xmd_copy = stream.get_xmd() assert xmd_copy - assert 'outer_key' in xmd - assert 'scalar' in xmd['outer_key'] - assert 'inner_key' in xmd['outer_key'][1] - assert xmd['outer_key'][1]['inner_key'] == 'another_scalar' + assert 'outer_key' in xmd_copy + assert 'scalar' in xmd_copy['outer_key'] + assert 'inner_key' in xmd_copy['outer_key'][1] + assert xmd_copy['outer_key'][1]['inner_key'] == 'another_scalar' def test_upgrade(self): v1_stream = Modulemd.ModuleStreamV1.new("SuperModule", "latest") @@ -1129,6 +1129,38 @@ def test_validate_buildafter(self): "%s/modulemd/v2/tests/test_data/buildafter/invalid_key.yaml" % (os.getenv('MESON_SOURCE_ROOT')), True) + def test_unicode_desc(self): + # Test a valid module stream with unicode in the description + stream = Modulemd.ModuleStream.read_file( + "%s/modulemd/v2/tests/test_data/stream_unicode.yaml" % + (os.getenv('MESON_SOURCE_ROOT')), True, '', '') + + self.assertIsNotNone(stream) + self.assertTrue(stream.validate()) + + def test_xmd_issue_274(self): + # Test a valid module stream with unicode in the description + stream = Modulemd.ModuleStream.read_file( + "%s/modulemd/v2/tests/test_data/stream_unicode.yaml" % + (os.getenv('MESON_SOURCE_ROOT')), True, '', '') + + # In this bug, we were getting a traceback attemping to call + # get_xmd() more than once on the same stream. There were subtle + # memory issues at play here. + if '_overrides_module' in dir(Modulemd): + # The XMD python tests can only be run against the installed lib + # because the overrides that translate between python and GVariant + # must be installed in /usr/lib/python*/site-packages/gi/overrides + # or they are not included when importing Modulemd + + xmd = stream.get_xmd() + mbs_xmd = stream.get_xmd()['mbs'] + mbs_xmd2 = stream.get_xmd()['mbs'] + + else: + stream.get_xmd() + stream.get_xmd() + if __name__ == '__main__': unittest.main() diff --git a/modulemd/v2/tests/test-modulemd-modulestream.c b/modulemd/v2/tests/test-modulemd-modulestream.c index 7d32f121a..13926a8f6 100644 --- a/modulemd/v2/tests/test-modulemd-modulestream.c +++ b/modulemd/v2/tests/test-modulemd-modulestream.c @@ -911,6 +911,52 @@ module_stream_v2_test_rpm_map (ModuleStreamFixture *fixture, g_assert_true (modulemd_rpm_map_entry_equals (entry, retrieved_entry)); } +static void +module_stream_v2_test_unicode_desc (void) +{ + g_autoptr (ModulemdModuleStream) stream = NULL; + g_autofree gchar *path = NULL; + g_autoptr (GError) error = NULL; + + /* Test a module stream with unicode in description */ + path = g_strdup_printf ("%s/modulemd/v2/tests/test_data/stream_unicode.yaml", + g_getenv ("MESON_SOURCE_ROOT")); + g_assert_nonnull (path); + + stream = modulemd_module_stream_read_file (path, TRUE, NULL, NULL, &error); + g_assert_nonnull (stream); + g_assert_no_error (error); +} + + +static void +module_stream_v2_test_xmd_issue_274 (void) +{ + g_autoptr (ModulemdModuleStream) stream = NULL; + g_autofree gchar *path = NULL; + g_autoptr (GError) error = NULL; + GVariant *xmd1 = NULL; + GVariant *xmd2 = NULL; + + path = g_strdup_printf ("%s/modulemd/v2/tests/test_data/stream_unicode.yaml", + g_getenv ("MESON_SOURCE_ROOT")); + g_assert_nonnull (path); + + stream = modulemd_module_stream_read_file (path, TRUE, NULL, NULL, &error); + g_assert_nonnull (stream); + g_assert_no_error (error); + g_assert_cmpint (modulemd_module_stream_get_mdversion (stream), + ==, + MD_MODULESTREAM_VERSION_ONE); + + xmd1 = + modulemd_module_stream_v1_get_xmd (MODULEMD_MODULE_STREAM_V1 (stream)); + xmd2 = + modulemd_module_stream_v1_get_xmd (MODULEMD_MODULE_STREAM_V1 (stream)); + + g_assert_true (xmd1 == xmd2); +} + int main (int argc, char *argv[]) @@ -1012,6 +1058,11 @@ main (int argc, char *argv[]) module_stream_v2_test_rpm_map, NULL); + g_test_add_func ("/modulemd/v2/modulestream/v2/unicode/description", + module_stream_v2_test_unicode_desc); + + g_test_add_func ("/modulemd/v2/modulestream/v2/xmd/issue274", + module_stream_v2_test_xmd_issue_274); return g_test_run (); } diff --git a/modulemd/v2/tests/test_data/stream_unicode.yaml b/modulemd/v2/tests/test_data/stream_unicode.yaml new file mode 100644 index 000000000..4d364f2a7 --- /dev/null +++ b/modulemd/v2/tests/test_data/stream_unicode.yaml @@ -0,0 +1,26 @@ +document: modulemd +version: 1 +data: + description: Fedora 28 traditional base ’ + name: platform + license: + module: [MIT] + profiles: + buildroot: + rpms: [bash, bzip2, coreutils, cpio, diffutils, fedora-release, findutils, gawk, + gcc, gcc-c++, grep, gzip, info, make, patch, redhat-rpm-config, rpm-build, + sed, shadow-utils, tar, unzip, util-linux, which, xz] + srpm-buildroot: + rpms: [bash, fedora-release, fedpkg-minimal, gnupg2, redhat-rpm-config, rpm-build, + shadow-utils] + stream: f28 + summary: Fedora 28 traditional base + version: 3 + context: 00000000 + xmd: + mbs: + buildrequires: {} + commit: virtual + requires: {} + mse: true + koji_tag: module-f28-build