Commit 614adf8a authored by Philip Withnall's avatar Philip Withnall

gvdb: Fix error handling in gvdb_table_new()

The documentation was unclear about what error codes would be returned
on attempting to open an empty or corrupt GVDB file. Previous versions
of the documentation incorrectly said that corrupt GVDB files were
considered equivalent to empty ones.

A recent commit has clarified the documentation to include its error
handling behaviour.

Update the two users of GVDB within GLib, GResource and GSettingsSource,
to follow this change, and add unit tests for them both.

Other users of the GVDB copylib will need to update their copy and make
appropriate changes if they have bugs in their handling of this
situation. dconf is one example of this. GVDB should be updated from
https://gitlab.gnome.org/GNOME/gvdb.
Signed-off-by: Philip Withnall's avatarPhilip Withnall <withnall@endlessm.com>

#1454
parent c652d45c
......@@ -512,6 +512,19 @@ g_resource_new_from_table (GvdbTable *table)
return resource;
}
static void
g_resource_error_from_gvdb_table_error (GError **g_resource_error,
GError *gvdb_table_error /* (transfer full) */)
{
if (g_error_matches (gvdb_table_error, G_FILE_ERROR, G_FILE_ERROR_INVAL))
g_set_error_literal (g_resource_error,
G_RESOURCE_ERROR, G_RESOURCE_ERROR_INTERNAL,
gvdb_table_error->message);
else
g_propagate_error (g_resource_error, g_steal_pointer (&gvdb_table_error));
g_clear_error (&gvdb_table_error);
}
/**
* g_resource_new_from_data:
* @data: A #GBytes
......@@ -528,6 +541,8 @@ g_resource_new_from_table (GvdbTable *table)
* Otherwise this function will internally create a copy of the memory since
* GLib 2.56, or in older versions fail and exit the process.
*
* If @data is empty or corrupt, %G_RESOURCE_ERROR_INTERNAL will be returned.
*
* Returns: (transfer full): a new #GResource, or %NULL on error
*
* Since: 2.32
......@@ -538,6 +553,7 @@ g_resource_new_from_data (GBytes *data,
{
GvdbTable *table;
gboolean unref_data = FALSE;
GError *local_error = NULL;
if (((guintptr) g_bytes_get_data (data, NULL)) % sizeof (gpointer) != 0)
{
......@@ -546,13 +562,16 @@ g_resource_new_from_data (GBytes *data,
unref_data = TRUE;
}
table = gvdb_table_new_from_bytes (data, TRUE, error);
table = gvdb_table_new_from_bytes (data, TRUE, &local_error);
if (unref_data)
g_bytes_unref (data);
if (table == NULL)
return NULL;
{
g_resource_error_from_gvdb_table_error (error, g_steal_pointer (&local_error));
return NULL;
}
return g_resource_new_from_table (table);
}
......@@ -568,6 +587,11 @@ g_resource_new_from_data (GBytes *data,
* If you want to use this resource in the global resource namespace you need
* to register it with g_resources_register().
*
* If @filename is empty or the data in it is corrupt,
* %G_RESOURCE_ERROR_INTERNAL will be returned. If @filename doesn’t exist, or
* there is an error in reading it, an error from g_mapped_file_new() will be
* returned.
*
* Returns: (transfer full): a new #GResource, or %NULL on error
*
* Since: 2.32
......@@ -577,10 +601,14 @@ g_resource_load (const gchar *filename,
GError **error)
{
GvdbTable *table;
GError *local_error = NULL;
table = gvdb_table_new (filename, FALSE, error);
table = gvdb_table_new (filename, FALSE, &local_error);
if (table == NULL)
return NULL;
{
g_resource_error_from_gvdb_table_error (error, g_steal_pointer (&local_error));
return NULL;
}
return g_resource_new_from_table (table);
}
......
......@@ -267,6 +267,9 @@ g_settings_schema_source_unref (GSettingsSchemaSource *source)
* Generally, you should set @trusted to %TRUE for files installed by the
* system and to %FALSE for files in the home directory.
*
* In either case, an empty file or some types of corruption in the file will
* result in %G_FILE_ERROR_INVAL being returned.
*
* If @parent is non-%NULL then there are two effects.
*
* First, if g_settings_schema_source_lookup() is called with the
......
......@@ -2353,6 +2353,18 @@ test_schema_source (void)
g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_NOENT);
g_clear_error (&error);
/* Test error handling of corrupt compiled files. */
source = g_settings_schema_source_new_from_directory ("schema-source-corrupt", parent, TRUE, &error);
g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL);
g_assert_null (source);
g_clear_error (&error);
/* Test error handling of empty compiled files. */
source = g_settings_schema_source_new_from_directory ("schema-source-empty", parent, TRUE, &error);
g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL);
g_assert_null (source);
g_clear_error (&error);
/* create a source with the parent */
source = g_settings_schema_source_new_from_directory ("schema-source", parent, TRUE, &error);
g_assert_no_error (error);
......@@ -2770,6 +2782,12 @@ main (int argc, char *argv[])
if (!g_test_subprocess ())
{
GError *local_error = NULL;
/* A GVDB header is 6 guint32s, and requires a magic number in the first
* two guint32s. A set of zero bytes of a greater length is considered
* corrupt. */
const guint8 gschemas_compiled_corrupt[sizeof (guint32) * 7] = { 0, };
backend_set = g_getenv ("GSETTINGS_BACKEND") != NULL;
g_setenv ("XDG_DATA_DIRS", ".", TRUE);
......@@ -2821,6 +2839,21 @@ main (int argc, char *argv[])
"--schema-file=" SRCDIR "/org.gtk.schemasourcecheck.gschema.xml",
NULL, NULL, &result, NULL));
g_assert (result == 0);
g_remove ("schema-source-corrupt/gschemas.compiled");
g_mkdir ("schema-source-corrupt", 0777);
g_file_set_contents ("schema-source-corrupt/gschemas.compiled",
(const gchar *) gschemas_compiled_corrupt,
sizeof (gschemas_compiled_corrupt),
&local_error);
g_assert_no_error (local_error);
g_remove ("schema-source-empty/gschemas.compiled");
g_mkdir ("schema-source-empty", 0777);
g_file_set_contents ("schema-source-empty/gschemas.compiled",
"", 0,
&local_error);
g_assert_no_error (local_error);
}
g_test_add_func ("/gsettings/basic", test_basic);
......
......@@ -317,6 +317,45 @@ test_resource_data_unaligned (void)
g_resource_unref (resource);
}
/* Test error handling for corrupt GResource files (specifically, a corrupt
* GVDB header). */
static void
test_resource_data_corrupt (void)
{
/* A GVDB header is 6 guint32s, and requires a magic number in the first two
* guint32s. A set of zero bytes of a greater length is considered corrupt. */
static const guint8 data[sizeof (guint32) * 7] = { 0, };
GBytes *bytes = NULL;
GResource *resource = NULL;
GError *local_error = NULL;
bytes = g_bytes_new_static (data, sizeof (data));
resource = g_resource_new_from_data (bytes, &local_error);
g_bytes_unref (bytes);
g_assert_error (local_error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_INTERNAL);
g_assert_null (resource);
g_clear_error (&local_error);
}
/* Test handling for empty GResource files. They should also be treated as
* corrupt. */
static void
test_resource_data_empty (void)
{
GBytes *bytes = NULL;
GResource *resource = NULL;
GError *local_error = NULL;
bytes = g_bytes_new_static (NULL, 0);
resource = g_resource_new_from_data (bytes, &local_error);
g_bytes_unref (bytes);
g_assert_error (local_error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_INTERNAL);
g_assert_null (resource);
g_clear_error (&local_error);
}
static void
test_resource_registered (void)
{
......@@ -785,6 +824,8 @@ main (int argc,
g_test_add_func ("/resource/file-path", test_resource_file_path);
g_test_add_func ("/resource/data", test_resource_data);
g_test_add_func ("/resource/data_unaligned", test_resource_data_unaligned);
g_test_add_func ("/resource/data-corrupt", test_resource_data_corrupt);
g_test_add_func ("/resource/data-empty", test_resource_data_empty);
g_test_add_func ("/resource/registered", test_resource_registered);
g_test_add_func ("/resource/manual", test_resource_manual);
g_test_add_func ("/resource/manual2", test_resource_manual2);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment