From 9849f2eff7c65049b3b2a3a8cb2cad427d2f1102 Mon Sep 17 00:00:00 2001 From: Jehan Date: Sat, 21 Feb 2026 14:18:42 +0100 Subject: [PATCH] app, pdb: factorize code for allowed filters. The code to ban some filters for non-destructive usage was duplicated in a PDB file and in the XCF load code. Additionally to combining these 2 codes into a single gimp_gegl_op_nde_allowed(), this commit also moves part of the logic into gimp_gegl_op_blacklisted() which improves the following: * It used to be possible to create filters for hidden operations which were not returned by gimp_drawable_filter_operation_get_available(), such as "gegl:color" or "gimp:equalize", which would create all sorts of problems. Now trying to create these filters through the API will not work and will properly warn with an explicit error message. I do not consider this an API break since the filters were not returned in the available lists and therefore were not considered usable. Anyone who would have used any of these hidden filters was just going around a weakness in our implementation. * We make sure that our lists of allowed/forbidden filters are consistent across usages. * When getting the list of filters with gimp_gegl_get_op_classes(), we don't need to do an additional validation step (as we were doing until now in the PDB call). This is meant to imply that all returned operations were already validated. --- app/gegl/gimp-gegl-utils.c | 128 +++++++++++++++++++++++++++------ app/gegl/gimp-gegl-utils.h | 2 + app/pdb/drawable-filter-cmds.c | 61 +--------------- app/xcf/xcf-load.c | 69 +++++------------- pdb/groups/drawable_filter.pdb | 63 +--------------- 5 files changed, 129 insertions(+), 194 deletions(-) diff --git a/app/gegl/gimp-gegl-utils.c b/app/gegl/gimp-gegl-utils.c index 10c398b99d..a4efede83e 100644 --- a/app/gegl/gimp-gegl-utils.c +++ b/app/gegl/gimp-gegl-utils.c @@ -31,6 +31,7 @@ #include "gimp-gegl-types.h" +#include "core/gimperror.h" #include "core/gimppattern.h" #include "core/gimpprogress.h" @@ -43,7 +44,8 @@ static gboolean gimp_gegl_op_blacklisted (const gchar *name, const gchar *categories, - gboolean block_gimp_ops); + gboolean block_gimp_ops, + GError **error); static GList * gimp_gegl_get_op_subclasses (GType type, GList *classes, gboolean block_gimp_ops); @@ -51,6 +53,10 @@ static gint gimp_gegl_compare_op_names (GeglOperationClass *a, GeglOperationClass *b); +/* Comes from gegl/operation/gegl-operations.h which is not public. */ +GType gegl_operation_gtype_from_name (const gchar *name); + + /* public functions */ GList * @@ -67,6 +73,34 @@ gimp_gegl_get_op_classes (gboolean block_gimp_ops) return operations; } +gboolean +gimp_gegl_op_nde_allowed (const gchar *name, + GError **error) +{ + GType op_type; + GeglOperationClass *klass; + const gchar *categories; + + g_return_val_if_fail (error != NULL && *error == NULL, FALSE); + + if (name == NULL || strlen (name) == 0) + { + g_set_error_literal (error, GIMP_ERROR, GIMP_FAILED, "the filter has no name."); + return FALSE; + } + else if (! gegl_has_operation (name)) + { + g_set_error (error, GIMP_ERROR, GIMP_FAILED, "the filter \"%s\" is not installed.", name); + return FALSE; + } + + op_type = gegl_operation_gtype_from_name (name); + klass = GEGL_OPERATION_CLASS (g_type_class_ref (op_type)); + categories = gegl_operation_class_get_key (klass, "categories"); + + return ! gimp_gegl_op_blacklisted (name, categories, FALSE, error); +} + GType gimp_gegl_get_op_enum_type (const gchar *operation, const gchar *property) @@ -474,9 +508,10 @@ gimp_gegl_buffer_set_extent (GeglBuffer *buffer, /* private functions */ static gboolean -gimp_gegl_op_blacklisted (const gchar *name, - const gchar *categories_str, - gboolean block_gimp_ops) +gimp_gegl_op_blacklisted (const gchar *name, + const gchar *categories_str, + gboolean block_gimp_ops, + GError **error) { static const gchar * const category_blacklist[] = { @@ -511,6 +546,7 @@ gimp_gegl_op_blacklisted (const gchar *name, "gegl:map-relative", /* pointless */ "gegl:matting-global", /* used in the foreground select tool */ "gegl:matting-levin", /* used in the foreground select tool */ + "gegl:nop", /* pointless */ "gegl:opacity", /* poinless */ "gegl:pack", /* pointless */ "gegl:path", @@ -526,44 +562,90 @@ gimp_gegl_op_blacklisted (const gchar *name, "gegl:wavelet-blur", /* we use gimp's op wavelet-decompose */ }; - gchar **categories; + GType op_type; gint i; /* Operations with no name are abstract base classes */ if (! name) return TRUE; + g_return_val_if_fail (error == NULL || *error == NULL, TRUE); + /* use this flag to include all ops for testing */ if (g_getenv ("GIMP_TESTING_NO_GEGL_BLACKLIST")) return FALSE; if (block_gimp_ops && g_str_has_prefix (name, "gimp")) - return TRUE; + { + g_set_error_literal (error, GIMP_ERROR, GIMP_FAILED, + "Filters in namespace \"gimp:\" are hidden."); + return TRUE; + } for (i = 0; i < G_N_ELEMENTS (name_blacklist); i++) { if (! strcmp (name, name_blacklist[i])) - return TRUE; + { + g_set_error_literal (error, GIMP_ERROR, GIMP_FAILED, + "The filter is hidden."); + return TRUE; + } } - if (! categories_str) - return FALSE; - - categories = g_strsplit (categories_str, ":", 0); - - for (i = 0; i < G_N_ELEMENTS (category_blacklist); i++) + if (g_strcmp0 (name, "gegl:gegl") == 0) { - gint j; - - for (j = 0; categories[j]; j++) - if (! strcmp (categories[j], category_blacklist[i])) - { - g_strfreev (categories); - return TRUE; - } + if (g_getenv ("GIMP_ALLOW_GEGL_GRAPH_LAYER_EFFECT") == NULL) + { + g_set_error (error, GIMP_ERROR, GIMP_FAILED, + "%s\n%s", "The filter is unsafe.", + "For development purpose, set environment variable GIMP_ALLOW_GEGL_GRAPH_LAYER_EFFECT."); + return TRUE; + } + else + { + return FALSE; + } } - g_strfreev (categories); + if (categories_str) + { + gchar **categories; + + categories = g_strsplit (categories_str, ":", 0); + + for (i = 0; i < G_N_ELEMENTS (category_blacklist); i++) + { + gint j; + + for (j = 0; categories[j]; j++) + if (! strcmp (categories[j], category_blacklist[i])) + { + g_strfreev (categories); + g_set_error (error, GIMP_ERROR, GIMP_FAILED, + "Filters from category \"%s\" are hidden.", + category_blacklist[i]); + return TRUE; + } + } + + g_strfreev (categories); + } + + op_type = gegl_operation_gtype_from_name (name); + /* Forbid filters which directly write into files. These should + * not be creatable through GIMP UI, but just in case someone + * builds one such XCF file (maliciously or by mistake/through a + * bug), let's prevent this filter to overwrite random files on + * load. + * Most of these sink ops are in group "output", but double-checking + * is not a bad thing here. + */ + if (g_type_is_a (op_type, GEGL_TYPE_OPERATION_SINK)) + { + g_set_error_literal (error, GIMP_ERROR, GIMP_FAILED, + "The filter is unsafe."); + return TRUE; + } return FALSE; } @@ -589,7 +671,7 @@ gimp_gegl_get_op_subclasses (GType type, categories = gegl_operation_class_get_key (klass, "categories"); - if (! gimp_gegl_op_blacklisted (klass->name, categories, block_gimp_ops)) + if (! gimp_gegl_op_blacklisted (klass->name, categories, block_gimp_ops, NULL)) classes = g_list_prepend (classes, klass); for (i = 0; i < n_ops; i++) diff --git a/app/gegl/gimp-gegl-utils.h b/app/gegl/gimp-gegl-utils.h index 20d284b2b4..d832845320 100644 --- a/app/gegl/gimp-gegl-utils.h +++ b/app/gegl/gimp-gegl-utils.h @@ -22,6 +22,8 @@ GList * gimp_gegl_get_op_classes (gboolean block_gimp_ops); +gboolean gimp_gegl_op_nde_allowed (const gchar *name, + GError **error); GType gimp_gegl_get_op_enum_type (const gchar *operation, const gchar *property); diff --git a/app/pdb/drawable-filter-cmds.c b/app/pdb/drawable-filter-cmds.c index 75592ac1af..2f48fc32f6 100644 --- a/app/pdb/drawable-filter-cmds.c +++ b/app/pdb/drawable-filter-cmds.c @@ -56,58 +56,6 @@ #include "gimp-intl.h" -static gboolean -validate_operation_name (const gchar *operation_name, - GError **error) -{ - GType op_type; - - /* Comes from gegl/operation/gegl-operations.h which is not public. */ - GType gegl_operation_gtype_from_name (const gchar *name); - - op_type = gegl_operation_gtype_from_name (operation_name); - - /* Using the same rules as in xcf_load_effect () for plug-in created - * effects. - */ - if (g_type_is_a (op_type, GEGL_TYPE_OPERATION_SINK)) - { - g_set_error (error, GIMP_PDB_ERROR, GIMP_PDB_ERROR_INVALID_ARGUMENT, - "%s: the filter \"%s\" is unsafe.", - G_STRFUNC, operation_name); - - return FALSE; - } - else if (g_strcmp0 (operation_name, "gegl:gegl") == 0 && - g_getenv ("GIMP_ALLOW_GEGL_GRAPH_LAYER_EFFECT") == NULL) - { - g_set_error (error, GIMP_PDB_ERROR, GIMP_PDB_ERROR_INVALID_ARGUMENT, - "%s: the filter \"gegl:gegl\" is unsafe.\n" - "For development purpose, set environment variable GIMP_ALLOW_GEGL_GRAPH_LAYER_EFFECT.", - G_STRFUNC); - - return FALSE; - } - - if (g_strcmp0 (operation_name, "gegl:nop") == 0) - { - g_set_error_literal (error, GIMP_PDB_ERROR, GIMP_PDB_ERROR_INVALID_ARGUMENT, - "The filter \"gegl:nop\" is useless and not allowed."); - return FALSE; - } - - if (! gegl_has_operation (operation_name)) - { - g_set_error (error, GIMP_PDB_ERROR, GIMP_PDB_ERROR_INVALID_ARGUMENT, - "%s: the filter \"%s\" is not installed.", - G_STRFUNC, operation_name); - - return FALSE; - } - - return TRUE; -} - static GimpValueArray * drawable_filter_id_is_valid_invoker (GimpProcedure *procedure, Gimp *gimp, @@ -158,7 +106,7 @@ drawable_filter_new_invoker (GimpProcedure *procedure, if (success) { - if (validate_operation_name (operation_name, error)) + if (gimp_gegl_op_nde_allowed (operation_name, error)) { GeglNode *operation = gegl_node_new (); @@ -745,9 +693,6 @@ drawable_filter_operation_get_available_invoker (GimpProcedure *procedur { GeglOperationClass *op_klass = op->data; - if (!validate_operation_name (op_klass->name, NULL)) - continue; - g_strv_builder_add (builder, op_klass->name); } @@ -780,7 +725,7 @@ drawable_filter_operation_get_details_invoker (GimpProcedure *procedure, if (success) { - if (validate_operation_name (operation_name, error)) + if (gimp_gegl_op_nde_allowed (operation_name, error)) { GStrvBuilder *names_builder = NULL; GeglOperationClass *klass = NULL; @@ -858,7 +803,7 @@ drawable_filter_operation_get_pspecs_invoker (GimpProcedure *procedure, if (success) { - if (validate_operation_name (operation_name, error)) + if (gimp_gegl_op_nde_allowed (operation_name, error)) { GParamSpec **specs = NULL; guint n_specs = 0; diff --git a/app/xcf/xcf-load.c b/app/xcf/xcf-load.c index 70f4503d9a..2fe6d692e3 100644 --- a/app/xcf/xcf-load.c +++ b/app/xcf/xcf-load.c @@ -35,6 +35,7 @@ #include "gegl/gimp-babl.h" #include "gegl/gimp-gegl-tile-compat.h" +#include "gegl/gimp-gegl-utils.h" #include "core/gimp.h" #include "core/gimpcontainer.h" @@ -4151,9 +4152,6 @@ xcf_load_channel (XcfInfo *info, return NULL; } -/* Comes from gegl/operation/gegl-operations.h which is not public. */ -GType gegl_operation_gtype_from_name (const gchar *name); - static FilterData * xcf_load_effect (XcfInfo *info, GimpImage *image, @@ -4163,7 +4161,7 @@ xcf_load_effect (XcfInfo *info, GimpChannel *effect_mask = NULL; goffset mask_offset = 0; gchar *string; - GType op_type; + GError *error = NULL; filter = g_new0 (FilterData, 1); @@ -4179,37 +4177,23 @@ xcf_load_effect (XcfInfo *info, xcf_read_string (info, &string, 1); filter->operation_name = string; - op_type = gegl_operation_gtype_from_name (filter->operation_name); - if (g_type_is_a (op_type, GEGL_TYPE_OPERATION_SINK)) - { - /* Forbid filters which directly write into files. These should - * not be creatable through GIMP UI, but just in case someone - * builds one such XCF file (maliciously or by mistake/through a - * bug), let's prevent this filter to overwrite random files on - * load. - */ - filter->unsupported_operation = TRUE; - - gimp_message (info->gimp, G_OBJECT (info->progress), - GIMP_MESSAGE_WARNING, - /* TODO: localize after string freeze. */ - "XCF Warning: the \"%s\" (%s) filter is " - "unsafe. It was discarded.", - filter->name, filter->operation_name); - - return filter; - } - else if (g_strcmp0 (filter->operation_name, "gegl:gegl") == 0 && - g_getenv ("GIMP_ALLOW_GEGL_GRAPH_LAYER_EFFECT") == NULL) + if (! gimp_gegl_op_nde_allowed (filter->operation_name, &error)) { filter->unsupported_operation = TRUE; - gimp_message (info->gimp, G_OBJECT (info->progress), - GIMP_MESSAGE_WARNING, - /* TODO: localize after string freeze. */ - "XCF Warning: the \"%s\" (%s) filter is unsafe. It was discarded.\n" - "For development purpose, set environment variable GIMP_ALLOW_GEGL_GRAPH_LAYER_EFFECT.", - filter->name, filter->operation_name); + if (filter->name) + gimp_message (info->gimp, G_OBJECT (info->progress), + GIMP_MESSAGE_WARNING, + /* TODO: localize after string freeze. */ + "XCF Warning: the filter \"%s\" (%s) was discarded. %s", + filter->name, filter->operation_name, error->message); + else + gimp_message (info->gimp, G_OBJECT (info->progress), + GIMP_MESSAGE_WARNING, + /* TODO: localize after string freeze. */ + "XCF Warning: an unnamed filter (%s) was discarded. %s", + filter->operation_name, error->message); + g_clear_error (&error); return filter; } @@ -4220,27 +4204,6 @@ xcf_load_effect (XcfInfo *info, filter->op_version = string; } - if (! gegl_has_operation (filter->operation_name) || - ! g_strcmp0 (filter->operation_name, "gegl:nop")) - { - filter->unsupported_operation = TRUE; - - if (! g_strcmp0 (filter->operation_name, "gegl:nop")) - gimp_message (info->gimp, G_OBJECT (info->progress), - GIMP_MESSAGE_WARNING, - "XCF Warning: A filter was saved as a " - "gegl:nop. This should not happen. Please " - "report this to the developers."); - else - gimp_message (info->gimp, G_OBJECT (info->progress), - GIMP_MESSAGE_WARNING, - "XCF Warning: the \"%s\" (%s) filter is " - "not installed. It was discarded.", - filter->name, filter->operation_name); - - return filter; - } - if (filter->op_version && g_strcmp0 (gegl_operation_get_op_version (filter->operation_name), filter->op_version) != 0) diff --git a/pdb/groups/drawable_filter.pdb b/pdb/groups/drawable_filter.pdb index 8cda729c1e..f97c673d86 100644 --- a/pdb/groups/drawable_filter.pdb +++ b/pdb/groups/drawable_filter.pdb @@ -75,7 +75,7 @@ HELP %invoke = ( code => <<'CODE' { - if (validate_operation_name (operation_name, error)) + if (gimp_gegl_op_nde_allowed (operation_name, error)) { GeglNode *operation = gegl_node_new (); @@ -686,9 +686,6 @@ HELP { GeglOperationClass *op_klass = op->data; - if (!validate_operation_name (op_klass->name, NULL)) - continue; - g_strv_builder_add (builder, op_klass->name); } @@ -743,7 +740,7 @@ HELP %invoke = ( code => <<'CODE' { -if (validate_operation_name (operation_name, error)) + if (gimp_gegl_op_nde_allowed (operation_name, error)) { GStrvBuilder *names_builder = NULL; GeglOperationClass *klass = NULL; @@ -824,7 +821,7 @@ HELP %invoke = ( code => <<'CODE' { - if (validate_operation_name (operation_name, error)) + if (gimp_gegl_op_nde_allowed (operation_name, error)) { GParamSpec **specs = NULL; guint n_specs = 0; @@ -912,60 +909,6 @@ CODE ); } -$extra{app}->{code} = <<'CODE'; -static gboolean -validate_operation_name (const gchar *operation_name, - GError **error) -{ - GType op_type; - - /* Comes from gegl/operation/gegl-operations.h which is not public. */ - GType gegl_operation_gtype_from_name (const gchar *name); - - op_type = gegl_operation_gtype_from_name (operation_name); - - /* Using the same rules as in xcf_load_effect () for plug-in created - * effects. - */ - if (g_type_is_a (op_type, GEGL_TYPE_OPERATION_SINK)) - { - g_set_error (error, GIMP_PDB_ERROR, GIMP_PDB_ERROR_INVALID_ARGUMENT, - "%s: the filter \"%s\" is unsafe.", - G_STRFUNC, operation_name); - - return FALSE; - } - else if (g_strcmp0 (operation_name, "gegl:gegl") == 0 && - g_getenv ("GIMP_ALLOW_GEGL_GRAPH_LAYER_EFFECT") == NULL) - { - g_set_error (error, GIMP_PDB_ERROR, GIMP_PDB_ERROR_INVALID_ARGUMENT, - "%s: the filter \"gegl:gegl\" is unsafe.\n" - "For development purpose, set environment variable GIMP_ALLOW_GEGL_GRAPH_LAYER_EFFECT.", - G_STRFUNC); - - return FALSE; - } - - if (g_strcmp0 (operation_name, "gegl:nop") == 0) - { - g_set_error_literal (error, GIMP_PDB_ERROR, GIMP_PDB_ERROR_INVALID_ARGUMENT, - "The filter \"gegl:nop\" is useless and not allowed."); - return FALSE; - } - - if (! gegl_has_operation (operation_name)) - { - g_set_error (error, GIMP_PDB_ERROR, GIMP_PDB_ERROR_INVALID_ARGUMENT, - "%s: the filter \"%s\" is not installed.", - G_STRFUNC, operation_name); - - return FALSE; - } - - return TRUE; -} -CODE - @headers = qw(